Bug 238869 - www/chromium: sndio input implementation
Summary: www/chromium: sndio input implementation
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: amd64 Any
: --- Affects Many People
Assignee: Carlos J. Puga Medina
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-06-28 19:39 UTC by zielonka michal
Modified: 2019-07-29 09:37 UTC (History)
5 users (show)

See Also:
bugzilla: maintainer-feedback? (chromium)


Attachments
sndio_input.cc sndio_input.h (8.72 KB, patch)
2019-06-28 19:39 UTC, zielonka michal
no flags Details | Diff
chromium-sndio.diff (2.13 KB, patch)
2019-07-28 11:01 UTC, Tobias Kortkamp
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description zielonka michal 2019-06-28 19:39:54 UTC
Created attachment 205403 [details]
sndio_input.cc sndio_input.h

I had a problem with missing implementation for recording audio for sndiod enabled in chromium port.

The alsa option often had made for me problem with lost audio input. I've checked if there is a proposal for filling implementation for sndio input, but unfortunately I couldn't find it.

So I tried to make it myself, and the patch with working for me version I attached here.

/u/ports# git remote  get-url origin
https://github.com/trueos/trueos-ports
/u/ports# git branch 
* trueos-master
Comment 1 Tobias Kortkamp freebsd_committer 2019-06-29 03:16:23 UTC
OpenBSD has imported an input implementation a couple of days ago:
https://github.com/openbsd/ports/commit/a82a036f9349ae5e0eec05738cdd2c570ff5dd35

So we would get that by syncing with them.  A patch that applies it
against the FreeBSD ports tree is available here:
https://github.com/t6/freebsd-ports-sndio/commit/f213ba25a3460ed6f8e858f04f5592fca8edb7d8.patch
Comment 2 zielonka michal 2019-06-29 21:03:13 UTC
I was to slow :). I agree that their implementation looks good. But I added also setting volume which is missed there.

 Do you think that I can make rebase to PR to T6?

 Also I have question, will it be better commit changes by github and PRs? I'm waiting now for account in phabricator from several days. But I'm a truly little confused which way is better: mailing list, bugzilla, phabricator or github :)
Comment 3 Tobias Kortkamp freebsd_committer 2019-06-30 03:38:16 UTC
(In reply to zielonka michal from comment #2)
> I was to slow :). I agree that their implementation looks good. But I added
> also setting volume which is missed there.
> 
>  Do you think that I can make rebase to PR to T6?

I would suggest to rebase and then send your changes to the OpenBSD
devs (i.e., ratchov@openbsd.org and robert@openbsd.org) to review
and integrate.

>  Also I have question, will it be better commit changes by github and PRs?
> I'm waiting now for account in phabricator from several days. But I'm a
> truly little confused which way is better: mailing list, bugzilla,
> phabricator or github :)

IMHO Bugzilla is the first choice.  The other things should ideally
be accompanied by bugs on Bugzilla for tracking progress.
Comment 4 zielonka michal 2019-06-30 05:17:24 UTC
thanks and I close this bug :)
Comment 5 Tobias Kortkamp freebsd_committer 2019-06-30 06:25:23 UTC
(In reply to zielonka michal from comment #4)
> thanks and I close this bug :)

It should stay open for tracking of the issue until this is resolved by us
actually importing an sndio input implementation into our ports tree.
Comment 6 Rene Ladan freebsd_committer 2019-07-08 19:41:58 UTC
I merged this into into the GitHub branch, see https://github.com/gliaskos/freebsd-chromium/pull/157
Comment 7 commit-hook freebsd_committer 2019-07-09 08:21:47 UTC
A commit references this bug:

Author: cpm
Date: Tue Jul  9 08:20:57 UTC 2019
New revision: 506266
URL: https://svnweb.freebsd.org/changeset/ports/506266

Log:
  www/chromium: Add support for audio recording using sndio

  PR:		238869
  Submitted by:	Zielonka Michal <zielonka.michal@gmail.com>, tobik
  Obtained from:	https://github.com/t6/freebsd-ports-sndio/commit/f213ba25a3460ed6f8e858f04f5592fca8edb7d8
  MFH:		2019Q3

Changes:
  head/www/chromium/files/sndio_input.cc
  head/www/chromium/files/sndio_input.h
  head/www/chromium/files/sndio_output.cc
  head/www/chromium/files/sndio_output.h
Comment 8 commit-hook freebsd_committer 2019-07-17 18:47:13 UTC
A commit references this bug:

Author: cpm
Date: Wed Jul 17 18:47:03 UTC 2019
New revision: 506812
URL: https://svnweb.freebsd.org/changeset/ports/506812

Log:
  MFH: r506266

  www/chromium: Add support for audio recording using sndio

  PR:		238869
  Submitted by:	Zielonka Michal <zielonka.michal@gmail.com>, tobik
  Obtained from:	https://github.com/t6/freebsd-ports-sndio/commit/f213ba25a3460ed6f8e858f04f5592fca8edb7d8

  Approved by:	ports-secteam (joneum)

Changes:
_U  branches/2019Q3/
  branches/2019Q3/www/chromium/files/sndio_input.cc
  branches/2019Q3/www/chromium/files/sndio_input.h
  branches/2019Q3/www/chromium/files/sndio_output.cc
  branches/2019Q3/www/chromium/files/sndio_output.h
Comment 9 Carlos J. Puga Medina freebsd_committer 2019-07-26 18:38:48 UTC
Reopen the PR until we find out why it causes audio/video sync problems in chromium.
Comment 10 Tobias Kortkamp freebsd_committer 2019-07-28 11:01:13 UTC
Created attachment 206112 [details]
chromium-sndio.diff

Here's the patch that fixes a/v sync issues as requested.

This has been committed to the OpenBSD Ports tree already.
Note that this is slightly different than what I sent to jrm@ since
another bug was found after that.  Please wait a little with committing
this since I'd like to test this first too.
Comment 11 Tobias Kortkamp freebsd_committer 2019-07-28 16:42:17 UTC
(In reply to Tobias Kortkamp from comment #10)
Seems fine. Please go ahead.
Comment 12 commit-hook freebsd_committer 2019-07-28 18:06:45 UTC
A commit references this bug:

Author: cpm
Date: Sun Jul 28 18:05:34 UTC 2019
New revision: 507520
URL: https://svnweb.freebsd.org/changeset/ports/507520

Log:
  www/chromium: Fix audio/video synchronization bug after r506266

  PR:		238869
  Submitted by:	tobik
  Reported by:	jrm
  MFH:		2019Q3

Changes:
  head/www/chromium/files/sndio_input.cc
  head/www/chromium/files/sndio_output.cc
Comment 13 Carlos J. Puga Medina freebsd_committer 2019-07-28 18:08:19 UTC
(In reply to Tobias Kortkamp from comment #11)

Committed! Thanks for your help Tobias.
Comment 14 Jonathan Chen 2019-07-28 19:07:31 UTC
Doesn't this merit a PORTREVISION bump?
Comment 15 Carlos J. Puga Medina freebsd_committer 2019-07-28 20:55:58 UTC
(In reply to Jonathan Chen from comment #14)

There is no need to increase PORTREVISION because SNDIO is not an option enabled by default.
Comment 16 Tobias Kortkamp freebsd_committer 2019-07-29 00:52:39 UTC
(In reply to Carlos J. Puga Medina from comment #13)
> (In reply to Tobias Kortkamp from comment #11)
> 
> Committed! Thanks for your help Tobias.

Thanks for committing the fix, Carlos.

(In reply to Carlos J. Puga Medina from comment #15)
> (In reply to Jonathan Chen from comment #14)
> 
> There is no need to increase PORTREVISION because SNDIO is not an option
> enabled by default.

This is wrong.  Of course it needs a PORTREVISION bump.  That it
is not a default option does not matter.  How are build tools
supposed to know that Chromium needs a rebuild without it?  As it
stands chromium-75.0.3770.142 with SNDIO=on has broken a/v sync and
users must rebuild it.  But that does not happen automatically
without a PORTREVISION bump.

Also see the Porter's Handbook which is very specific about this
(second paragraph):
https://www.freebsd.org/doc/en/books/porters-handbook/makefile-naming.html#makefile-portrevision

I've bumped it in ports r507545.  Please remember to MFH the bump
to 2019Q3 too.
Comment 17 commit-hook freebsd_committer 2019-07-29 09:37:19 UTC
A commit references this bug:

Author: cpm
Date: Mon Jul 29 09:36:34 UTC 2019
New revision: 507564
URL: https://svnweb.freebsd.org/changeset/ports/507564

Log:
  MFH: r507520

  www/chromium: Fix audio/video synchronization bug after r506266

  PR:		238869
  Submitted by:	tobik
  Reported by:	jrm

  Approved by:	ports-secteam (miwi)

Changes:
_U  branches/2019Q3/
  branches/2019Q3/www/chromium/files/sndio_input.cc
  branches/2019Q3/www/chromium/files/sndio_output.cc