Bug#734100: Fixed in VLC 3.0

[Forest]

Remi Denis-Courmont wrote:

>VLC upstream has not approved any patchset on top of VLC 2.2, 

Yes, I know.  Their failure to address this bug is responsible for vlc being
badly broken for a long time now.  I would think this an appropriate reason
to apply a downstream patch.

>the upstream solution is very far from trivial to backport.

Yes, I know.  I saw it.

>At least, all the patches for VLC 2.2 that I´ve seen proposed are all known 
>broken. That is to say that they do fix the symptoms, but they introduce even 
>worse problems that _will_ break VLC for other people.

I appreciate what you're saying, but have you examined my patchset?  It's
not quite the same as any other that I've seen, and I have yet to receive
any criticism of it at all.  If it will cause breakage, I'd like to know
specifically how, so I can either fix it or abandon the effort.  I'd hate to
think it was being prematurely dismissed.

___
pkg-multimedia-maintainers mailing list
pkg-multimedia-maintainers@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-multimedia-maintainers

Bug#734100: Fixed in VLC 3.0

[Forest]

Modestas, in DecoderPlayAudio(), your modified decoder.c patch fails to
unlock the mutex when the loop exits on !p_audio.  On my system, this causes
vlc to become unresponsive when a video ends.

I looked in to 557eaa06 and 5b2de769 as suggested by Rémi, but since they
depend on many other commits and don't appear necessary in my own testing, I
abandoned the effort.

I'm attaching my current patches for review.  One is adapted from git commit
6ae2905e, while the other is copied verbatim from 47f74a83.  As far as I can
tell, they fix the stuttering bug in vlc 2.2.2 and 2.2.4, without causing
new problems.

X-Git-Url: 
https://git.videolan.org/?p=vlc.git;a=blobdiff_plain;f=src%2Finput%2Fdecoder.c;h=fe3cd428c65c18bfbdadb55baf11521afdc2bfc7;hp=83aa5bf54e2c29ad93fae803117558e4fcd0f658;hb=6ae2905ef7fbc7de3a3a4a1bdf8ad6df46ce570a;hpb=5b2de76965ee8b1ab5e3257f8b6d71bbb4e9e3f9

--- a/src/input/decoder.c
+++ b/src/input/decoder.c
@@ -1162,7 +1162,10 @@
 b_paused = p_owner->b_paused;
 
 if (!p_audio)
+{
+vlc_mutex_unlock( _owner->lock );
 break;
+}
 
 /* */
 int i_rate = INPUT_RATE_DEFAULT;
@@ -1180,6 +1183,9 @@
 
 if( unlikely(p_owner->b_paused != b_paused) )
 continue; /* race with input thread? retry... */
+
+vlc_mutex_unlock( _owner->lock );
+
 if( p_aout == NULL )
 b_reject = true;
 
@@ -1199,7 +1205,6 @@
 
 break;
 }
-vlc_mutex_unlock( _owner->lock );
 }
 
 static void DecoderDecodeAudio( decoder_t *p_dec, block_t *p_block )
@@ -1961,11 +1966,10 @@
 
 /* Parameters changed, restart the aout */
 vlc_mutex_lock( _owner->lock );
-
-aout_DecDelete( p_owner->p_aout );
 p_owner->p_aout = NULL;
-
 vlc_mutex_unlock( _owner->lock );
+aout_DecDelete( p_owner->p_aout );
+
 input_resource_PutAout( p_owner->p_resource, p_aout );
 }
 
X-Git-Url: 
https://git.videolan.org/?p=vlc.git;a=blobdiff_plain;f=modules%2Faudio_output%2Falsa.c;h=4e9fd53592d048baa8b57f30df15ab5806139d07;hp=2d1f99e9cb743bca12c6bdf32cc84a92d07fda8b;hb=47f74a83c161173b0d15e95dab8ceb7c97de51b4;hpb=6ae2905ef7fbc7de3a3a4a1bdf8ad6df46ce570a

diff --git a/modules/audio_output/alsa.c b/modules/audio_output/alsa.c
index 2d1f99e..4e9fd53 100644
--- a/modules/audio_output/alsa.c
+++ b/modules/audio_output/alsa.c
@@ -495,6 +495,15 @@ static int Start (audio_output_t *aout, 
audio_sample_format_t *restrict fmt)
 }
 sys->rate = fmt->i_rate;
 
+#if 1 /* work-around for period-long latency outputs (e.g. PulseAudio): */
+param = AOUT_MIN_PREPARE_TIME;
+val = snd_pcm_hw_params_set_period_time_near (pcm, hw, , NULL);
+if (val)
+{
+msg_Err (aout, "cannot set period: %s", snd_strerror (val));
+goto error;
+}
+#endif
 /* Set buffer size */
 param = AOUT_MAX_ADVANCE_TIME;
 val = snd_pcm_hw_params_set_buffer_time_near (pcm, hw, , NULL);
@@ -503,14 +512,22 @@ static int Start (audio_output_t *aout, 
audio_sample_format_t *restrict fmt)
 msg_Err (aout, "cannot set buffer duration: %s", snd_strerror (val));
 goto error;
 }
-
-param = AOUT_MIN_PREPARE_TIME;
+#if 0
+val = snd_pcm_hw_params_get_buffer_time (hw, , NULL);
+if (val)
+{
+msg_Warn (aout, "cannot get buffer time: %s", snd_strerror(val));
+param = AOUT_MIN_PREPARE_TIME;
+}
+else
+param /= 2;
 val = snd_pcm_hw_params_set_period_time_near (pcm, hw, , NULL);
 if (val)
 {
 msg_Err (aout, "cannot set period: %s", snd_strerror (val));
 goto error;
 }
+#endif
 
 /* Commit hardware parameters */
 val = snd_pcm_hw_params (pcm, hw);
___
pkg-multimedia-maintainers mailing list
pkg-multimedia-maintainers@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-multimedia-maintainers

Bug#734100: Fixed in VLC 3.0

[Forest]

On Mon, 02 Nov 2015 21:16:48 +0200, Rémi Denis-Courmont wrote:

>At least 557eaa06 and 5b2de769 are needed too. I can´t really check deeper in 
>my free time.

Do you mean they're needed to completely fix the bug, or just in order to
make the other two patches to apply cleanly?

I refreshed and applied 6ae2905e and 47f74a83 to vlc 2.2.2 (the current
ubuntu release), and in my initial tests, the bug hasn't resurfaced yet.

For anyone interested in testing my build or grabbing my refreshed patch:
https://launchpad.net/~foresto/+archive/ubuntu/ubuntutweaks/

___
pkg-multimedia-maintainers mailing list
pkg-multimedia-maintainers@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-multimedia-maintainers

Bug#734100: Fixed in VLC 3.0

[Rémi Denis-Courmont]

On Monday 02 November 2015 00:34:47 Modestas Vainius wrote:
> The end result builds fine and seems to fix the problem on my machine. Not
> sure if patches break anything on 2.2.1 (i.e. if they need anything else
> from 3.0.0 to work properly). Rémi, maybe you could comment on that?

At least 557eaa06 and 5b2de769 are needed too. I can´t really check deeper in 
my free time.

-- 
Rémi Denis-Courmont
http://www.remlab.net/

___
pkg-multimedia-maintainers mailing list
pkg-multimedia-maintainers@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-multimedia-maintainers

Bug#734100: Fixed in VLC 3.0

[Rémi Denis-Courmont]

tags 734100 + fixed-upstream
thanks

This should be fixed in upstream VLC 3.0.
-- 
Rémi Denis-Courmont
http://www.remlab.net/

___
pkg-multimedia-maintainers mailing list
pkg-multimedia-maintainers@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-multimedia-maintainers

Bug#734100: Fixed in VLC 3.0

[Modestas Vainius]

Hello,

Sekmadienis 01 Lapkritis 2015 18:10:54 rašė:
> tags 734100 + fixed-upstream
> thanks
> 
> This should be fixed in upstream VLC 3.0.

Thanks a lot. Could this package be backported to current packages? This 
problem has been annoying me for a long time and lately it got only worse. 
Unfortunately, increasing dmix buffer has some unwanted side effects in other 
apps (audio-video desync).

-- 
Modestas Vainius 

___
pkg-multimedia-maintainers mailing list
pkg-multimedia-maintainers@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-multimedia-maintainers

Processed: Re: Bug#734100: Fixed in VLC 3.0

[Debian Bug Tracking System]

Processing commands for cont...@bugs.debian.org:

> reopen 734100
Bug #734100 {Done: Sebastian Ramacher } [vlc] vlc: Video 
stuttering with ALSA output
Bug reopened
Ignoring request to alter fixed versions of bug #734100 to the same values 
previously set
> thanks
Stopping processing here.

Please contact me if you need assistance.
-- 
734100: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=734100
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems

___
pkg-multimedia-maintainers mailing list
pkg-multimedia-maintainers@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-multimedia-maintainers

Bug#734100: Fixed in VLC 3.0

[Rémi Denis-Courmont]

reopen 734100
thanks

On Sunday 01 November 2015 22:03:41 Modestas Vainius wrote:
> Hello,
> 
> Sekmadienis 01 Lapkritis 2015 18:10:54 rašė:
> > tags 734100 + fixed-upstream
> > thanks
> > 
> > This should be fixed in upstream VLC 3.0.
> 
> Thanks a lot. Could this package be backported to current packages?

VLC 3.0 is nowhere near release at this point in time. This is up to the 
Debian Multimedia team, not me, but I would not advise uploading it to 
unstable. And I do not know if there are enough human resources to maintain an 
extra VLC 3.0 in experimental.

> This
> problem has been annoying me for a long time and lately it got only worse.
> Unfortunately, increasing dmix buffer has some unwanted side effects in
> other apps (audio-video desync).

Either those other apps are buggy, or dmix is buggy. The audio delay value 
should account for the dmix buffer.

Also, PulseAudio is better at mixing multiple apps.

-- 
Rémi Denis-Courmont
http://www.remlab.net/

___
pkg-multimedia-maintainers mailing list
pkg-multimedia-maintainers@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-multimedia-maintainers

Bug#734100: Fixed in VLC 3.0

[Modestas Vainius]

Hello,

Sekmadienis 01 Lapkritis 2015 22:43:17 rašė:
> reopen 734100
> thanks
> 
> On Sunday 01 November 2015 22:03:41 Modestas Vainius wrote:
> > Hello,
> > 
> > Sekmadienis 01 Lapkritis 2015 18:10:54 rašė:
> > > tags 734100 + fixed-upstream
> > > thanks
> > > 
> > > This should be fixed in upstream VLC 3.0.
> > 
> > Thanks a lot. Could this package be backported to current packages?
> 
> VLC 3.0 is nowhere near release at this point in time. This is up to the
> Debian Multimedia team, not me, but I would not advise uploading it to
> unstable. And I do not know if there are enough human resources to maintain
> an extra VLC 3.0 in experimental.

What I really meant was backporting of those patches to 2.2.1 packages. 
Patches seemed pretty "short" at the glance hence I tried to do that myself.

47f74a83c161173b0d15e95dab8ceb7c97de51b4.patch was strightforward, applies 
cleanly.

Unfortunately, 6ae2905ef7fbc7de3a3a4a1bdf8ad6df46ce570a wasn't that 
strightforward. Had to do some modifications: 1st hunk already there in 2.2.1, 
adapt DecoderPlayAudio hunks and remove the last 2 hunks (from DecoderThread 
routine) since those locks are not present there in 2.2.1 codebase. I attach 
both patches.

The end result builds fine and seems to fix the problem on my machine. Not 
sure if patches break anything on 2.2.1 (i.e. if they need anything else from 
3.0.0 to work properly). Rémi, maybe you could comment on that?

> 
> > This
> > problem has been annoying me for a long time and lately it got only worse.
> > Unfortunately, increasing dmix buffer has some unwanted side effects in
> > other apps (audio-video desync).
> 
> Either those other apps are buggy, or dmix is buggy. The audio delay value
> should account for the dmix buffer.
> 
> Also, PulseAudio is better at mixing multiple apps.

Well, yeah, but I kind of dislike PulseAudio since its inception. I'm slowly 
warming up to the idea that I may have to use it one day but that time has not 
come yet...

-- 
Modestas Vainius diff --git a/modules/audio_output/alsa.c b/modules/audio_output/alsa.c
index 2d1f99e..4e9fd53 100644
--- a/modules/audio_output/alsa.c
+++ b/modules/audio_output/alsa.c
@@ -495,6 +495,15 @@ static int Start (audio_output_t *aout, audio_sample_format_t *restrict fmt)
 }
 sys->rate = fmt->i_rate;
 
+#if 1 /* work-around for period-long latency outputs (e.g. PulseAudio): */
+param = AOUT_MIN_PREPARE_TIME;
+val = snd_pcm_hw_params_set_period_time_near (pcm, hw, , NULL);
+if (val)
+{
+msg_Err (aout, "cannot set period: %s", snd_strerror (val));
+goto error;
+}
+#endif
 /* Set buffer size */
 param = AOUT_MAX_ADVANCE_TIME;
 val = snd_pcm_hw_params_set_buffer_time_near (pcm, hw, , NULL);
@@ -503,14 +512,22 @@ static int Start (audio_output_t *aout, audio_sample_format_t *restrict fmt)
 msg_Err (aout, "cannot set buffer duration: %s", snd_strerror (val));
 goto error;
 }
-
-param = AOUT_MIN_PREPARE_TIME;
+#if 0
+val = snd_pcm_hw_params_get_buffer_time (hw, , NULL);
+if (val)
+{
+msg_Warn (aout, "cannot get buffer time: %s", snd_strerror(val));
+param = AOUT_MIN_PREPARE_TIME;
+}
+else
+param /= 2;
 val = snd_pcm_hw_params_set_period_time_near (pcm, hw, , NULL);
 if (val)
 {
 msg_Err (aout, "cannot set period: %s", snd_strerror (val));
 goto error;
 }
+#endif
 
 /* Commit hardware parameters */
 val = snd_pcm_hw_params (pcm, hw);
--- a/src/input/decoder.c
+++ b/src/input/decoder.c
@@ -1180,6 +1180,8 @@ static void DecoderPlayAudio( decoder_t
 
 if( unlikely(p_owner->b_paused != b_paused) )
 continue; /* race with input thread? retry... */
+vlc_mutex_unlock( _owner->lock );
+
 if( p_aout == NULL )
 b_reject = true;
 
@@ -1199,7 +1201,6 @@ static void DecoderPlayAudio( decoder_t
 
 break;
 }
-vlc_mutex_unlock( _owner->lock );
 }
 
 static void DecoderDecodeAudio( decoder_t *p_dec, block_t *p_block )
@@ -1961,11 +1962,10 @@ static int aout_update_format( decoder_t
 
 /* Parameters changed, restart the aout */
 vlc_mutex_lock( _owner->lock );
-
-aout_DecDelete( p_owner->p_aout );
 p_owner->p_aout = NULL;
-
 vlc_mutex_unlock( _owner->lock );
+aout_DecDelete( p_owner->p_aout );
+
 input_resource_PutAout( p_owner->p_resource, p_aout );
 }
 
___
pkg-multimedia-maintainers mailing list
pkg-multimedia-maintainers@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-multimedia-maintainers