Add gapless support
Gapless as a seamless transition between 2 audio media, this MR is not about cross-fading (not planned for VLC 4.0).
If gapless is enabled and possible, the vlc_player_t will load a new input_thread_t, in gapless mode, near the end of the current input_thread_t.
This input_thead_t in gapless mode will act as a normal input_thread_t except that it will be halted at the end of the buffering, waiting for a signal from the player to resume. The gapless preparation can be cancelled (if the user stop/seek/etc...).
The audio output is also modified to handle gapless. If an aout stream is notified to have a next stream playing (input_thread_t in gapless mode), it won't drain and stop at the end of the current stream.
I fixed the points discussed during the last VLC tech meeting https://code.videolan.org/videolan/vlc/-/wikis/Meetingreports/2020-10-04-VLC-Tech-Meeting-via-videoconf#gapless
- Gapless is only enabled if the input can be paced (Use it only for file-based access: file, smb, nfs...)
- Gapless is only enabled when playing only one audio ES (that drive the block == master)
- It can only work if the reported input time and length are correct
Merge request reports
Activity
changed milestone to %4.0
added Component::Core: Input label
- Resolved by Thomas Guillem
This MR depends on !43 (merged)
added MRStatus::Reviewable label
added MRStatus::InReview label and removed MRStatus::Reviewable label
added 23 commits
-
92ee9437...58ba0326 - 7 commits from branch
videolan:master
- 0ae55b2d - input: rename/move input_create_option
- afa44a55 - input: merge all Create functions
- eec9e786 - input: let the owner delete the input node
- 7614f17c - decoder: pass input_type directly
- 559d1a62 - aout: split aout_DecPlay
- 1dcfd284 - aout: add aout_FormatEquals
- 3c770672 - aout: add gapless support
- 918e8d1a - player: split vlc_player_InvalidateNextMedia
- 9dfa3c1e - input: add gapless support
- a8e21fc2 - input: add INPUT_EVENT_GAPLESS_NEXT
- b174cd09 - player: simplify vlc_player_OpenNextMedia
- 1bd791b5 - player: simplify media_stopped_action handling
- 65afc3da - player: only send event for the main input
- cef2d110 - player: add gapless support
- 5f698af8 - test: player: test gapless
- ccd54db9 - lib: add "gapless" option (enabled by default)
Toggle commit list-
92ee9437...58ba0326 - 7 commits from branch
- Resolved by Thomas Guillem
Since this is a huge patch set, the 24H/72H rules from the bot should not apply. I unresolve this thread to add more time.
- Resolved by Thomas Guillem
- Resolved by Thomas Guillem
added 49 commits
-
ccd54db9...ac14138a - 34 commits from branch
videolan:master
- be7ad393 - input: rename/move input_create_option
- 7c3887fc - input: merge all Create functions
- 89e64cc2 - input: let the owner delete the input node
- 410f4988 - decoder: pass input_type directly
- d6b3421d - aout: split aout_DecPlay
- 95d097e2 - aout: add gapless support
- a241c9b2 - player: split vlc_player_InvalidateNextMedia
- b29a7491 - input: add gapless support
- dcb69299 - input: add INPUT_EVENT_GAPLESS_NEXT
- 710c412d - player: simplify vlc_player_OpenNextMedia
- da56caf4 - player: simplify media_stopped_action handling
- 88b0aa93 - player: only send event for the main input
- 51df58f7 - player: add gapless support
- 3b50d2f3 - test: player: test gapless
- 3f0c995c - lib: add "gapless" option (enabled by default)
Toggle commit list-
ccd54db9...ac14138a - 34 commits from branch
added MRStatus::Acceptable label and removed MRStatus::InReview label
- Resolved by Thomas Guillem
- Resolved by Thomas Guillem
- Resolved by Thomas Guillem
685 else 686 return false; 687 } 688 } 689 690 return has_audio; 691 } 692 693 static void EsOutGaplessEventTimeChanged( es_out_t *out, vlc_tick_t time, 694 vlc_tick_t length ) 695 { 696 es_out_sys_t *p_sys = container_of(out, es_out_sys_t, out); 697 input_thread_t *p_input = p_sys->p_input; 698 699 /* Gapless is possible only if one AUDIO ES is selected and is the master 700 * source and if the input can be paced. */ I don't think you can evaluate the possibility of a parallel connection. If it causes a time out, buffers will have run under before it happens.
And this of course brings back the main original problem that this kind of invasive change with new waits and locks is guaranteed to cause new bugs and does not belong in 4.0.
added MRStatus::InReview label and removed MRStatus::Acceptable label
added MRStatus::NotCompliant label and removed MRStatus::InReview label
assigned to @tguillem
mentioned in issue #26378 (closed)
mentioned in merge request !1586 (merged)
mentioned in merge request !1654 (merged)
!1586 (merged) and !1654 (merged) are now merged.
added 3516 commits
-
3f0c995c...02293b1d - 3497 commits from branch
videolan:master
- 4a20aa07 - input: let the owner delete the input node
- ddb33720 - aout: initialize the stream members earlier
- 303c4017 - aout: rework the drained check
- 472585e7 - aout: change the drained atomic type
- d004f254 - aout: split vlc_aout_stream_Drain
- dc0dbc25 - aout: lock main_stream from events
- d0b6e48f - aout: add aout_FormatEquals
- f676daa6 - aout: add gapless support
- 5462c616 - inpout_resource: add gapless support
- 03c4e221 - input: add gapless support
- acfbb710 - input: add INPUT_EVENT_GAPLESS_NEXT
- 645c2813 - player: split vlc_player_InvalidateNextMedia
- 8fbf9acc - player: simplify vlc_player_OpenNextMedia
- 360b3da5 - player: simplify media_stopped_action handling
- a3e0dc79 - player: only send event for the main input
- be0a3b67 - player: add gapless support
- 26864518 - test: increase timeout
- 81cec535 - test: player: test gapless
- cdf54ded - lib: add "gapless" option (enabled by default)
Toggle commit list-
3f0c995c...02293b1d - 3497 commits from branch
added MRStatus::InReview label and removed MRStatus::NotCompliant label
mentioned in merge request !1782 (merged)
added 118 commits
-
cdf54ded...599191d3 - 93 commits from branch
videolan:master
- 7ade763b - aout: filters: use an intermediate struct to hold filters
- 4af65eaf - aout: filters: factorize filter destroy
- b6bbfab5 - aout: filters: only create a clock when necessary
- 6f4a9081 - aout: filters: move the vout ownership to aout_filters
- 656a5b7c - vout: add vout_ChangeClock
- a7cdb082 - aout: filters: add aout_FiltersChangeClock
- 3756b4b3 - input: let the owner delete the input node
- da866575 - aout: initialize the stream members earlier
- a746533b - aout: rework the drained check
- 86db8208 - aout: change the drained atomic type
- 3b9eee99 - aout: split vlc_aout_stream_Drain
- b1e8ba3c - aout: lock main_stream from events
- 881c8323 - aout: add aout_FormatEquals
- 4fba8b96 - aout: add gapless support
- 6a5d0aff - inpout_resource: add gapless support
- 747357fc - input: add gapless support
- cd7b9d50 - input: add INPUT_EVENT_GAPLESS_NEXT
- 2de872c9 - player: split vlc_player_InvalidateNextMedia
- 522fd37a - player: simplify vlc_player_OpenNextMedia
- dc2d4024 - player: simplify media_stopped_action handling
- ebb2ad46 - player: only send event for the main input
- 91fa8075 - player: add gapless support
- 0e731498 - test: increase timeout
- e10e8a2b - test: player: test gapless
- 2f669956 - lib: add "gapless" option (enabled by default)
Toggle commit list-
cdf54ded...599191d3 - 93 commits from branch
- Resolved by Thomas Guillem
Depends on !1782 (merged)
added 20 commits
- d86805a6 - aout: filters: add aout_FiltersCanSwitchStream
- ecab08eb - input: let the owner delete the input node
- 9635f61b - aout: initialize the stream members earlier
- 4f9fb149 - aout: rework the drained check
- f8f1c843 - aout: change the drained atomic type
- 38e0a0e6 - aout: split vlc_aout_stream_Drain
- 284def5b - aout: lock main_stream from events
- 4b6a7d76 - aout: add aout_FormatEquals
- add3e610 - aout: add gapless support
- f310e4d1 - inpout_resource: add gapless support
- bbfe5b62 - input: add gapless support
- ddab8b41 - input: add INPUT_EVENT_GAPLESS_NEXT
- 0443f9eb - player: split vlc_player_InvalidateNextMedia
- 20afe3e3 - player: simplify vlc_player_OpenNextMedia
- 10131c4b - player: simplify media_stopped_action handling
- cba1fe26 - player: only send event for the main input
- 29e6923d - player: add gapless support
- 418ee411 - test: increase timeout
- a5eaaa04 - test: player: test gapless
- d41e9542 - lib: add "gapless" option (enabled by default)
Toggle commit listadded 57 commits
-
d41e9542...b7b5fd49 - 33 commits from branch
videolan:master
- 278efe44 - aout: filters: use an intermediate struct to hold filters
- d48a8808 - aout: filters: factorize filter destroy
- f834379e - aout: filters: only create a clock when necessary
- 38891c2d - aout: filters: move the vout ownership to aout_filters
- 01a3529f - aout: filters: add aout_FiltersCanSwitchStream
- 483cf5b5 - input: let the owner delete the input node
- 5af48b8d - aout: initialize the stream members earlier
- 4a880ebb - aout: rework the drained check
- 141541ba - aout: change the drained atomic type
- 28f54005 - aout: split vlc_aout_stream_Drain
- f1be518e - aout: lock main_stream from events
- 67ae829d - aout: add aout_FormatEquals
- 677df9e3 - aout: add gapless support
- 8d32fe2d - inpout_resource: add gapless support
- 38973bb5 - input: add gapless support
- cb7eaefb - input: add INPUT_EVENT_GAPLESS_NEXT
- d237e8be - player: split vlc_player_InvalidateNextMedia
- 72c35720 - player: simplify vlc_player_OpenNextMedia
- 80b10b71 - player: simplify media_stopped_action handling
- 07f1a7e4 - player: only send event for the main input
- fb66a695 - player: add gapless support
- 615a1673 - test: increase timeout
- 07d3422b - test: player: test gapless
- e27947a0 - lib: add "gapless" option (enabled by default)
Toggle commit list-
d41e9542...b7b5fd49 - 33 commits from branch
added MRStatus::Acceptable label and removed MRStatus::InReview label
692 692 return NULL; 693 693 } 694 694 695 static int aout_FiltersPipelineCanSwitchStream(const struct aout_filter *tab, added MRStatus::InReview label and removed MRStatus::Acceptable label
- Resolved by Rémi Denis-Courmont
90 90 return &stream->instance->output; 91 91 } 92 92 93 static bool aout_FormatEquals(const audio_sample_format_t *f1, 94 const audio_sample_format_t *f2) 95 { 96 return f1->i_format == f2->i_format 97 && f1->i_rate == f2->i_rate 98 && f1->i_physical_channels == f2->i_physical_channels 99 && f1->i_chan_mode == f2->i_chan_mode 100 && f1->channel_type == f2->channel_type 101 && f1->i_bytes_per_frame == f2->i_bytes_per_frame 102 && f1->i_frame_length == f2->i_frame_length 103 && f1->i_bitspersample == f2->i_bitspersample 104 && f1->i_blockalign == f2->i_blockalign 105 && f1->i_channels == f2->i_channels; 82 82 * only lock or lock_hold to read them */ 83 83 struct vlc_list vout_rscs; 84 84 85 bool b_aout_busy; Multiple input thread exist for a long time since that's what happens with multiple media players. It's really about multiple input thread with the same input_resource and the same owner AFAIU.
But then, it might be easy to add tests to check whether only the limited scope for gapless is used in gapless scenario and start improving how much we could trust this patchset.
I don't share your optimism, especially not considering that the code comes from the one member of the TC who notoriously seems never to step back to think about the unintended consequences, is already single-handedly responsible for the single most disruptive push in the timeline of 4.0, and keeps doing feature work when they have one of the largest number of outstanding assigned 4.0 bugs (and I'm not the only person who's noted that much).
Aside of multiple inputs, and of personal reputations, I'm much more concerned about adding a new wait (in the decoder).
I don't share your optimism, especially not considering that the code comes from the one member of the TC who notoriously seems never to step back to think about the unintended consequences, is already single-handedly responsible for the single most disruptive push in the timeline of 4.0
Both developer-wise and company-wise, the break and complete instability of the video output is a much more disruptive change than the Qt interface change. The GUI disruption was also really about a player/playlist change which fixed a huge number of bugs and simplified the development for newcomers like me.
keeps doing feature work when they have one of the largest number of outstanding assigned 4.0 bugs (and I'm not the only person who's noted that much).
Afaik, Thomas is also fixing a large number of bugs both on his free time and time at Videolabs, even some of those you reported: #26716 (closed).
TBH, sharing Ad hominem attacks publicly on developers for multiple years each time you want to force a technical point on Thomas isn't fair nor decent. You could have just written "I don't share your optimism, the complexity introduced by the changes for 4.0 on the decoder is already high enough" to share your point, and use personal private way to vent your frustration or whatever it is.
The strong suspicion that the author is evidently overcommitted is highly relevant actually. As is the observed propensity of some developers to implement invasive/disruptive features with less consideration for unintended side effects.
Your complaining about alleged ad hominem is not exactly conducive to honest and fair review practices, and quality assurance.
Edited by Rémi Denis-CourmontWhile I get your reluctance to add features in 4.0, attacking a fellow TC member is not acceptable. If you think that there is an issue with this MR please elaborate with a case where it will actually break and the TC will vote but blocking a MR on a hunch isn't really fair.
The conditions to enable gapless are harsh and you said it the test will fail most of the time and I think that it should be disabled by default but if somebody wants to try it as an experimental feature. I say why not? It might raise issues that we wouldn't have expected for the buffering rework.
If the gapless feature let Thomas scratch his own nose, I don't see an issue with commitment. Opensource dev have to have their own pleasure or they'll lose all motivation.
What the hell is happening here? How am I hypocritical? I'm just saying what I think. If the feature is disabled by default why should it be discarded? 4.0 will be broken anyway in many case we don't foresee and we'll have to rework it anyway for 5.0 so the sooner we have a clear picture of the issues we have to expect the better and it's not like Thomas isn't going to maintain it. TC was created to avoid exactly this kind of useless drama, please take some time to cool down and raise a technical issue with the TC.
Edited by Denis CharmetThere is no mention of making this optional and disabled by default anywhere. You're literally blaming me for not accepting something that you just made up, Denis.
A member of TC is supposed to look out for problems and it doesn't take long to think suspicious non-ideal scenarii here, especially with the wait thing. But apparently you're more into criticising reviewers and fellow TC members than doing actual technical review.
I am very very very disappointed in all three of you here. But eh, it's much easier to gang on the last active volunteer than take a balanced standpoint.
Edited by Rémi Denis-CourmontWell I said that it should be disabled by default in my first post and I still stand by it. If I failed to pass it as a necessity I apologize for not being clear enough but I was proposing a middleground solution.
It's late and I'm still at work so maybe I failed to think of easily suspicious case but if you have any pointer I'll gladly take those.
It looks that what seemed to me a tentative of de-escalation and a begin of mediation only made it worse. My words conveyed way more than I expected. I assume that responsibility, I'm sincerily sorry and I apologize for it. My take was lazy and having slept on it here is a new one.
I really think that gapless is an interesting feature but I also know from my current attempts at making something similar at $dayjob that it can fail spectacularily.
With all due respect to Thomas' work it would be my personal preference that this MR is put on hold at least until 4.0 is out and stabilized.
Here are a few reasons why:
- It's a core modification that wasn't planned for 4.0 and it's already late as it is.
- I expect the clock to fail miserably on some cases at wrap-around and I'm still not happy about the float-> double trick so resetting it every time is safer.
- From my personal experience you cannot expect decoders especially hardware ones to properly notify you when they are drained on time.
- You can not expect filters especially resamplers to drain properly without injecting some safe samples
- The harsh conditions that are imposed to enable gapless and that keep the changeset "simple enough" will create inconsistent behaviour for the users and we'd have to educate them.
- I suspect some tricky race conditions between user inputs and stream switch.
- I wonder how it behaves for very short files like beeps.
- I think that gapless needs a proper rearchitecture and that it belongs to 5.0
I will nonetheless review it more accurately this week-end and see by myself, but if I fail to find technical issues, my personal preferences will stay what they are. IMO, my absence of contributions makes me illegetimate to block another dev's work.
Edited by Denis Charmet
813 { 814 vlc_tick_t drain_deadline = 815 atomic_load_explicit(&stream->drain_deadline, memory_order_relaxed); 816 817 if (drain_deadline != VLC_TICK_INVALID) 818 return vlc_tick_now() >= drain_deadline; 819 else 820 return atomic_load_explicit(&stream->drained, memory_order_relaxed) == 1; 821 } 822 823 static void stream_Drain(vlc_aout_stream *stream) 673 824 { 674 825 audio_output_t *aout = aout_stream_aout(stream); 675 826 676 if (aout->drain == NULL) 827 if (aout->drain) nit, but maybe keep the
!= NULL
:)Edited by Alexandre Janniaux
added 106 commits
-
e27947a0...63854be2 - 86 commits from branch
videolan:master
- 5461930b - aout: filters: add aout_FiltersCanSwitchStream
- afc05caf - input: let the owner delete the input node
- 9ad2b548 - aout: initialize the stream members earlier
- 7792abed - aout: rework the drained check
- 355786bf - aout: change the drained atomic type
- 03c0ed8d - aout: split vlc_aout_stream_Drain
- f4b4120f - aout: lock main_stream from events
- 1cc65da2 - aout: add aout_FormatEquals
- 007f7481 - aout: add gapless support
- 416d0921 - inpout_resource: add gapless support
- ce3f5738 - input: add gapless support
- 1a558f43 - input: add INPUT_EVENT_GAPLESS_NEXT
- e4457809 - player: split vlc_player_InvalidateNextMedia
- 43046e87 - player: simplify vlc_player_OpenNextMedia
- d159c088 - player: simplify media_stopped_action handling
- 0594220a - player: only send event for the main input
- 0c9470ad - player: add gapless support
- 10cddc94 - test: increase timeout
- a7fff77a - test: player: test gapless
- cc7ac5ae - lib: add "gapless" option (enabled by default)
Toggle commit list-
e27947a0...63854be2 - 86 commits from branch
749 917 stream->original_pts = VLC_TICK_INVALID; 750 918 } 919 920 void vlc_aout_stream_SwitchGapless(vlc_aout_stream *main, vlc_aout_stream *gapless) 921 { 922 aout_owner_t *owner = aout_stream_owner(main); 923 924 if (!aout_FiltersCanSwitchStream(main->filters)) 925 { 926 audio_output_t *aout = aout_stream_aout(main); 927 msg_Warn(aout, "gapless transition: need to restart aout filters"); 928 vlc_aout_stream_RequestRestart(gapless, AOUT_RESTART_FILTERS); 929 } 930 else 931 { 932 gapless->filters = main->filters; Data race:
WARNING: ThreadSanitizer: data race (pid=43142) Read of size 8 at 0x7b4400059d90 by thread T19: #0 stream_Reset ../../src/audio_output/dec.c:132 (libvlccore.so.9+0xa20f4) #1 vlc_aout_stream_Delete ../../src/audio_output/dec.c:316 (libvlccore.so.9+0xa2feb) #2 DeleteDecoder ../../src/input/decoder.c:2117 (libvlccore.so.9+0x5e02d) #3 vlc_input_decoder_Delete ../../src/input/decoder.c:2337 (libvlccore.so.9+0x6117a) #4 EsOutDestroyDecoder ../../src/input/es_out.c:2470 (libvlccore.so.9+0x66ce7) #5 EsOutUnselectEs ../../src/input/es_out.c:2639 (libvlccore.so.9+0x6b86b) #6 EsOutVaPrivControlLocked ../../src/input/es_out.c:3797 (libvlccore.so.9+0x70b64) #7 EsOutPrivControl ../../src/input/es_out.c:4096 (libvlccore.so.9+0x71dc2) #8 es_out_vaPrivControl ../../src/input/es_out.h:105 (libvlccore.so.9+0x76dcc) #9 es_out_PrivControl ../../src/input/es_out.h:112 (libvlccore.so.9+0x76dcc) #10 CmdExecutePrivControl ../../src/input/es_out_timeshift.c:1809 (libvlccore.so.9+0x76f27) #11 PrivControlLocked ../../src/input/es_out_timeshift.c:799 (libvlccore.so.9+0x7774a) #12 PrivControl ../../src/input/es_out_timeshift.c:858 (libvlccore.so.9+0x77976) #13 es_out_vaPrivControl ../../src/input/es_out.h:105 (libvlccore.so.9+0x799dc) #14 es_out_PrivControl ../../src/input/es_out.h:112 (libvlccore.so.9+0x799dc) #15 es_out_SetMode ../../src/input/es_out.h:119 (libvlccore.so.9+0x821e3) #16 End ../../src/input/input.c:1458 (libvlccore.so.9+0x821e3) #17 Run ../../src/input/input.c:485 (libvlccore.so.9+0x82745) Previous write of size 8 at 0x7b4400059d90 by thread T9: #0 vlc_aout_stream_SwitchGapless ../../src/audio_output/dec.c:932 (libvlccore.so.9+0xa4165) #1 aout_OutputDelete ../../src/audio_output/output.c:813 (libvlccore.so.9+0xa9fae) #2 vlc_aout_stream_Delete ../../src/audio_output/dec.c:318 (libvlccore.so.9+0xa3007) #3 DeleteDecoder ../../src/input/decoder.c:2117 (libvlccore.so.9+0x5e02d) #4 vlc_input_decoder_Delete ../../src/input/decoder.c:2337 (libvlccore.so.9+0x6117a) #5 EsOutDestroyDecoder ../../src/input/es_out.c:2470 (libvlccore.so.9+0x66ce7) #6 EsOutUnselectEs ../../src/input/es_out.c:2639 (libvlccore.so.9+0x6b86b) #7 EsOutVaPrivControlLocked ../../src/input/es_out.c:3797 (libvlccore.so.9+0x70b64) #8 EsOutPrivControl ../../src/input/es_out.c:4096 (libvlccore.so.9+0x71dc2) #9 es_out_vaPrivControl ../../src/input/es_out.h:105 (libvlccore.so.9+0x76dcc) #10 es_out_PrivControl ../../src/input/es_out.h:112 (libvlccore.so.9+0x76dcc) #11 CmdExecutePrivControl ../../src/input/es_out_timeshift.c:1809 (libvlccore.so.9+0x76f27) #12 PrivControlLocked ../../src/input/es_out_timeshift.c:799 (libvlccore.so.9+0x7774a) #13 PrivControl ../../src/input/es_out_timeshift.c:858 (libvlccore.so.9+0x77976) #14 es_out_vaPrivControl ../../src/input/es_out.h:105 (libvlccore.so.9+0x799dc) #15 es_out_PrivControl ../../src/input/es_out.h:112 (libvlccore.so.9+0x799dc) #16 es_out_SetMode ../../src/input/es_out.h:119 (libvlccore.so.9+0x821e3) #17 End ../../src/input/input.c:1458 (libvlccore.so.9+0x821e3) #18 Run ../../src/input/input.c:485 (libvlccore.so.9+0x82745)
222 222 } 223 223 224 224 /* Audio output */ 225 audio_output_t *input_resource_GetAout( input_resource_t *p_resource ) 225 audio_output_t *input_resource_GetAout( input_resource_t *p_resource, bool gapless ) 226 226 { 227 227 audio_output_t *p_aout; 228 228 229 229 vlc_mutex_lock( &p_resource->lock_hold ); 230 230 p_aout = p_resource->p_aout; 231 231 232 if( p_aout == NULL || p_resource->b_aout_busy ) 232 if( gapless ) It might be a silly idea, but wouldn't it be "easier" to hide gapless from the core by making the resource provide in gapless aout/vout wrappers?
It could abuse the fact that output modules' "write" function are considered to be blocking to wait for the resource to be actually available?
For example the wrapped create would return the current format of the actual aout, play would block until it's given the actual aout, get_time would return -1, flush would unblock everything and drain would just give the resource to the next aout wrapper (and possibly the vout to the next vout wrapper).
This way the rest of the core would'nt have to change it would be only the resource and the player which would know about it. With this you might be able to keep a visualization filter and maybe skip the harsh equality conditions by letting the filter negociation do the work.
The previous gap-less implementation (by me) more or less did that and it was an unmitigated disaster that was quickly reverted.
And that was before VLC drained properly. Now that VLC does drain properly, gap-less is fundamentally impossible without some awareness from the audio output and/or the audio decoder.
613 stream->gapless_transition = GAPLESS_TRANSITION_OK; 614 vlc_mutex_unlock(&owner->lock); 615 616 if (wait_drain) 617 { 618 /* Wait for the stream to be drained, if the transition failed */ 619 vlc_tick_t drain_deadline = 620 atomic_load_explicit(&stream->drain_deadline, memory_order_relaxed); 621 if (drain_deadline == VLC_TICK_INVALID) 622 { 623 while (atomic_load_explicit(&stream->drained, 624 memory_order_relaxed) == 0) 625 vlc_atomic_wait(&stream->drained, 0); 626 } 627 else 628 vlc_tick_wait(drain_deadline); My issue with here is that vlc_tick_wait doesn't take into account a possible user made pause that could occur during the dain. And it's a bug that seems to already exist in the core for aout without drain.
Maybe the change pause should be able to update the drain deadline but then what would happen to aout who don't know how to pause. Not being able to pause might be a condition to disable gapless.
699 { 700 vlc_clock_t *clock = tab[i].clock; 701 vout_thread_t *vout = tab[i].vout; 702 /* We can't change the clock or the vout used by a filter */ 703 if (clock != NULL && vout != NULL) 704 return false; 705 } 706 707 return true; 708 } 709 710 bool aout_FiltersCanSwitchStream(const aout_filters_t *filters) 711 { 712 assert(filters->clock_source); 713 714 return aout_FiltersPipelineCanSwitchStream(filters->tab, filters->count); 1221 1272 static void 1222 1273 vlc_player_SetPause(vlc_player_t *player, bool pause) 1223 1274 { 1224 1275 struct vlc_player_input *input = vlc_player_get_input_locked(player); This will be a problem during the drain/gapless transition and should most likely also apply to the gapless stream and you might want to check at the player level that it won't break during:
- add slave
- seek (especially backward)
- establishing loops
- titles stuff
- program changes
Basically all the player input control. Should rate apply during/after the switch?
added MRStatus::NotCompliant label and removed MRStatus::InReview label