Skip to content
Snippets Groups Projects

Add gapless support

Open Thomas Guillem requested to merge tguillem/vlc:gapless/0 into master
12 unresolved threads

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

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Rémi Denis-Courmont
  • Did not review final patches given preceding issues.

  • Thomas Guillem added 49 commits

    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)

    Compare with previous version

  • Thomas Guillem resolved all threads

    resolved all threads

  • Rémi Denis-Courmont
  • Rémi Denis-Courmont
  • Rémi Denis-Courmont
  • 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. */
    • That's not sufficient. For instance, HTTP can usually but not always be opened in parallel. It breaks cookies and some connection number limits.

    • Author Maintainer

      Indeed, PACED is not enough in that case, so I should add a new stream/access control then.

    • Author Maintainer

      If a parallel connection is not possible (in gapless mode), the player will try again when the main media is terminated. Is that enough?

      If not, I will add this new stream control.

    • 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.

    • Author Maintainer

      OK, I will add a new control for accesses to tell if // opening is possible.

    • Please register or sign in to reply
  • mentioned in issue #26378 (closed)

  • Thomas Guillem mentioned in merge request !1586 (merged)

    mentioned in merge request !1586 (merged)

  • Thomas Guillem mentioned in merge request !1654 (merged)

    mentioned in merge request !1654 (merged)

  • Thomas Guillem added 3516 commits

    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)

    Compare with previous version

  • Thomas Guillem mentioned in merge request !1782 (merged)

    mentioned in merge request !1782 (merged)

  • Thomas Guillem added 118 commits

    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)

    Compare with previous version

  • Thomas Guillem added 20 commits

    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)

    Compare with previous version

  • Thomas Guillem added 57 commits

    added 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)

    Compare with previous version

  • Thomas Guillem resolved all threads

    resolved all threads

  • 692 692 return NULL;
    693 693 }
    694 694
    695 static int aout_FiltersPipelineCanSwitchStream(const struct aout_filter *tab,
  • 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;
    • This cannot work if there are more than one active audio tracks (which is already possible with custom streaming output), and you don't know if the resource-tracked audio output matches the gaped pair of tracks.

    • Please register or sign in to reply
  • 104 104 vlc_cond_t wait_request;
    105 105 vlc_cond_t wait_acknowledge;
    106 106 vlc_cond_t wait_fifo; /* TODO: merge with wait_acknowledge */
    107 vlc_cond_t wait_gapless;
    • Again, this does not belong in 4.0. Having two concurrent input threads is guaranteed to break stuff.

    • Author Maintainer

      ???

    • What? We've never had more than one input thread per playlist/player. I'd be surprised if there was not some subtle bug.

      Also, buffering reworks are supposed to be in 5.0, not 4.0.

    • Author Maintainer

      I did not know for which commit/line this comment was address.

      We already support parallel access since VLC 3.0 via taglib, no issues so far.

    • We have supported parallel access and demux for many many years, for subtitles and other secondary sources. But gap-less necessarily requires multiple "input thread" objects and state machine for a single player instance and that is new.

    • 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-Courmont
    • While 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 Charmet
    • There 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-Courmont
    • Well 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
    • Please register or sign in to reply
  • 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)
  • Thomas Guillem added 106 commits

    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)

    Compare with previous version

  • Thomas Guillem
    Thomas Guillem @tguillem started a thread on commit 007f7481
  • 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;
    • Author Maintainer

      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)
      
    • Please register or sign in to reply
  • Denis Charmet
  • 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.

    • Please register or sign in to reply
  • Denis Charmet
  • 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.

    • Please register or sign in to reply
  • Denis Charmet
  • 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);
    • Doesn't keeping the filters cost more in gapless consistency than just restart from a clean state? If they are properly drained it should work.

    • Author Maintainer

      That was in my first proposal. Indeed, with filters drain, the transition was smooth.

      Maybe, we should go back to this first solution.

    • Please register or sign in to reply
  • Denis Charmet
  • 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?

    • Please register or sign in to reply
  • Please register or sign in to reply
    Loading