Bug 215867

Summary: www/firefox-esr: volume control don't work with SNDIO option
Product: Ports & Packages Reporter: Sergey <kpect>
Component: Individual Port(s)Assignee: freebsd-gecko (Nobody) <gecko>
Status: Closed FIXED    
Severity: Affects Many People CC: tobik
Priority: --- Flags: jbeich: maintainer-feedback+
Version: Latest   
Hardware: amd64   
OS: Any   
Attachments:
Description Flags
firefox-esr.diff
none
firefox-esr.diff none

Description Sergey 2017-01-08 08:07:19 UTC
Hello.

After switching from ALSA over to SNDIO option on firefox-esr 45.6.0, volume control stopped working on HTML 5 video player and on youtube video player. I can still hear the sound but can't make it louder/quieter or switch it off anymore.

$ uname -r
11.0-STABLE

Regards,
Sergey.
Comment 1 Sergey 2017-01-08 09:04:54 UTC
I've noticed also that when I pause the player and then resume, videocontent freezes until I drag the handle a bit, then video content resumes; meanwhile sound continues playing.
Comment 2 Tobias Kortkamp freebsd_committer 2017-01-11 15:58:40 UTC
Hi Sergey,

please check that sndiod is running.  See the pkg-message of audio/sndio:

    sysrc sndiod_enable=YES
    service sndiod start

When the daemon is not running you get no volume control in applications.
Comment 3 Sergey 2017-01-11 21:09:38 UTC
Hi Tobias,

I've done as you proposed, but now when sndiod is running video content has stopped playing. When I press pause/resume button the image is twitching. When I stop sndiod I can see the video content playing but symptoms are as depicted above.

Here are the options for the sndiod daemon:
/usr/local/bin/sndiod -f rsnd/0 -c 0:7 -j off -s default -m mon -s monitor

Regards,
Sergey.
Comment 4 Tobias Kortkamp freebsd_committer 2017-01-11 21:24:34 UTC
(In reply to Sergey from comment #3)
Believe it or not that sounds like progress :).  I don't think this is
a Firefox issue.  Is /dev/dsp0 (rsnd/0) a playback only device?
What's the content of /dev/sndstat?

If it is, try adding `-m play` to sndiod_flags i.e. set
sndiod_flags="-m play -f rsnd/0 -c 0:7 -j off -s default -m mon -s monitor"
in /etc/rc.conf.
Comment 5 Sergey 2017-01-12 16:53:15 UTC
(In reply to Tobias Kortkamp from comment #4)
Hi Tobias,
you are right, sound volume control works now pretty well!
Here is the /dev/dsp0 device. Not sure if it's a playback only device
# ll /dev/dsp0
crw-rw-rw-  1 root  wheel  0x86 Jan 12 19:44 /dev/dsp0
# cat /dev/sndstat
Installed devices:
pcm0: <Creative EMU10K1> (play/rec) default
No devices installed from userspace.

Thoght I still see the issue which is told about in Comment 1. When I pause the video clip and then resume afterwards sound starts to play but video freezes for some time and then resumes with some delay. Dragging the handle makes sound and video synchronous. (( 
ALSA libs haven't rendered such an effect.

Haven't you noticed the same effect with SNDIO?
Best Regards,
Sergey P.
Comment 6 Tobias Kortkamp freebsd_committer 2017-01-12 18:44:29 UTC
Created attachment 178800 [details]
firefox-esr.diff

(In reply to Sergey from comment #5)

> Haven't you noticed the same effect with SNDIO?

I was able to reproduce this with firefox-esr now.  The problem only
appears when using a local sndio server, which I haven't tried before.
Sndio support in firefox-esr has bugs and I've tried to backport some
changes from www/firefox.

Please try applying the attached patch to your ports tree and
recompile www/firefox-esr.  I've not tested it much, but it appears to
fix the problem.

For the record: www/firefox and www/seamonkey do not have this issue.
Comment 7 Sergey 2017-01-12 22:01:00 UTC
(In reply to Tobias Kortkamp from comment #6)
WOW Tobias! A magician you are!
You've fixed all the issues I had with Firefox-esr! Splendid work, let me thank you very much )

As for native Firefox, unfortunately I can't us it for the reason I've explained here: 
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=215879

At first I thought that was because of Greasemonky plugin together with viewtube script wich I use for high speed viewing and other bell and whistles but removing them didn't solve the issue. Though I have to say that I experimented only with alsa, sndio wasn't there at that moment yet. How do you think, will it work with sndio better that alsa does?

Regards
Comment 8 Tobias Kortkamp freebsd_committer 2017-01-13 10:26:27 UTC
Sergey, can you reopen this please?  IMO this should be committed to the ports tree.  I can't do this, so we have to hope that a committer does this for us.  But it's unlikely to happen when this bug is closed.

The current ESR version will be with us for a while longer and you can't be the only person to eventually run into this.
Comment 9 Sergey 2017-01-13 10:38:42 UTC
(In reply to Tobias Kortkamp from comment #8)
Ok no problem
Comment 10 Jan Beich freebsd_committer 2017-01-13 12:08:26 UTC
Comment on attachment 178800 [details]
firefox-esr.diff

Can you split into files/patch-bugNNN chunks for easy lookup on bugzilla.mozilla.org for more details or, at least, in which release landed? For example, files/patch-bug1153179 if you only intend to backport the specific one.

> -sndio_get_min_latency(cubeb * ctx, cubeb_stream_params params, uint32_t * latency_ms)
> +sndio_get_min_latency(cubeb * ctx, cubeb_stream_params params, uint32_t * latency_frames)
>  {
>    // XXX Not yet implemented.
> -  *latency_ms = 40;
> +  *latency_frames = 2048;

libcubeb only changed how latency is calculated in Firefox 50[1]. Do you really need it on 45.x ? If so adjust sio_par.appbufsz like upstream did. Maybe you want sndio fix[2] from Firefox 48 instead.

[1] https://github.com/kinetiknz/cubeb/commit/025b515bfed3 (API)
    https://github.com/kinetiknz/cubeb/commit/098b9f21bf99 (sndio)
    https://github.com/kinetiknz/cubeb/commit/46584e72bd5d (alsa)
    https://github.com/kinetiknz/cubeb/commit/645f58ed0094 (pulse)
[2] https://github.com/kinetiknz/cubeb/commit/658d7eba38d4
Comment 11 Tobias Kortkamp freebsd_committer 2017-01-13 13:59:35 UTC
Created attachment 178860 [details]
firefox-esr.diff

(In reply to Jan Beich (mail not working) from comment #10)

> libcubeb only changed how latency is calculated in Firefox 50[1]. Do you really need it on 45.x ?

No, that shouldn't be in there.

Fixes for bug 1153151 and 1153179 were committed at the same time [1],
I think both are needed to fix the problem here.  Just 1153151 isn't
enough.

> Can you split into files/patch-bugNNN chunks for easy lookup on bugzilla.mozilla.org for more details

Done.

[1] https://github.com/kinetiknz/cubeb/pull/79
Comment 12 commit-hook freebsd_committer 2017-01-13 18:00:09 UTC
A commit references this bug:

Author: jbeich
Date: Fri Jan 13 17:59:04 UTC 2017
New revision: 431408
URL: https://svnweb.freebsd.org/changeset/ports/431408

Log:
  www/firefox-esr: improve A/V sync with SNDIO=on

  PR:		215867
  Reported by:	Sergey <kpect@protonmail.com>
  Submitted by:	Tobias Kortkamp <t@tobik.me>
  Obtained from:	upstream
  MFH:		2017Q1

Changes:
  head/mail/thunderbird/Makefile
  head/mail/thunderbird/files/patch-bug1153151
  head/mail/thunderbird/files/patch-bug1153179
  head/www/firefox-esr/Makefile
  head/www/firefox-esr/files/patch-bug1153151
  head/www/firefox-esr/files/patch-bug1153179
  head/www/libxul/Makefile
  head/www/libxul/files/patch-bug1153151
  head/www/libxul/files/patch-bug1153179
Comment 13 commit-hook freebsd_committer 2017-01-14 00:12:40 UTC
A commit references this bug:

Author: jbeich
Date: Sat Jan 14 00:12:30 UTC 2017
New revision: 431453
URL: https://svnweb.freebsd.org/changeset/ports/431453

Log:
  MFH: r431408

  www/firefox-esr: improve A/V sync with SNDIO=on

  PR:		215867
  Reported by:	Sergey <kpect@protonmail.com>
  Submitted by:	Tobias Kortkamp <t@tobik.me>
  Obtained from:	upstream
  Approved by:	ports-secteam (feld)

Changes:
_U  branches/2017Q1/
  branches/2017Q1/mail/thunderbird/Makefile
  branches/2017Q1/mail/thunderbird/files/patch-bug1153151
  branches/2017Q1/mail/thunderbird/files/patch-bug1153179
  branches/2017Q1/www/firefox-esr/Makefile
  branches/2017Q1/www/firefox-esr/files/patch-bug1153151
  branches/2017Q1/www/firefox-esr/files/patch-bug1153179
  branches/2017Q1/www/libxul/Makefile
  branches/2017Q1/www/libxul/files/patch-bug1153151
  branches/2017Q1/www/libxul/files/patch-bug1153179
Comment 14 commit-hook freebsd_committer 2017-11-29 00:13:26 UTC
A commit references this bug:

Author: tobik
Date: Wed Nov 29 00:12:42 UTC 2017
New revision: 455086
URL: https://svnweb.freebsd.org/changeset/ports/455086

Log:
  www/palemoon: Update to 27.6.2

  - Add SNDIO option
  - Include bug fixes that improve A/V sync with SNDIO=on [1]
  - Allow armv6 build

  Changes:	http://www.palemoon.org/releasenotes.shtml
  PR:		223934, 215867 [1]
  Submitted by:	lichray@gmail.com (maintainer)
  Security:	6056bf68-f570-4e70-b740-b9f606971283

Changes:
  head/www/palemoon/Makefile
  head/www/palemoon/distinfo
  head/www/palemoon/files/patch-bug1153151
  head/www/palemoon/files/patch-bug1153179
  head/www/palemoon/files/patch-gfx_skia_moz.build