Linux Media Controller development
 help / color / mirror / Atom feed
* Re: [PATCH v2 26/31] sound/usb: Update ALSA driver to use Managed Media Controller API
From: Shuah Khan @ 2016-01-17 21:22 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: hans.verkuil, laurent.pinchart, clemens, sakari.ailus, javier,
	mchehab, alsa-devel, arnd, ricard.wanderlof, labbott,
	chehabrafael, misterpib, prabhakar.csengg, ricardo.ribalda,
	ruchandani.tina, takamichiho, tvboxspy, dominic.sacre, crope,
	julian, pierre-louis.bossart, corbet, joe, johan, dan.carpenter,
	pawel, p.zabel, perex, stefanr, inki.dae, jh1009.sung,
	k.kozlowski, kyungmin.park, m.szyprowski, sw0312.kim, elfring,
	linux-api, linux-kernel, linux-media, linuxbugs, gtmkramer,
	normalperson, daniel, Shuah Khan
In-Reply-To: <s5h1t9huc45.wl-tiwai@suse.de>

On 01/16/2016 02:06 AM, Takashi Iwai wrote:
> On Sat, 16 Jan 2016 01:38:27 +0100,
> Shuah Khan wrote:
>>
>> On 01/15/2016 06:38 AM, Takashi Iwai wrote:
>>> On Thu, 14 Jan 2016 20:52:28 +0100,
>>> Shuah Khan wrote:
>>>>
>>>> Change ALSA driver to use Managed Media Managed Controller
>>>> API to share tuner with DVB and V4L2 drivers that control
>>>> AU0828 media device.  Media device is created based on a
>>>> newly added field value in the struct snd_usb_audio_quirk.
>>>> Using this approach, the media controller API usage can be
>>>> added for a specific device. In this patch, Media Controller
>>>> API is enabled for AU0828 hw. snd_usb_create_quirk() will
>>>> check this new field, if set will create a media device using
>>>> media_device_get_devres() interface.
>>>>
>>>> media_device_get_devres() will allocate a new media device
>>>> devres or return an existing one, if it finds one.
>>>>
>>>> During probe, media usb driver could have created the media
>>>> device devres. It will then initialze (if necessary) and
>>>> register the media device if it isn't already initialized
>>>> and registered. Media device unregister is done from
>>>> usb_audio_disconnect().
>>>>
>>>> During probe, media usb driver could have created the
>>>> media device devres. It will then register the media
>>>> device if it isn't already registered. Media device
>>>> unregister is done from usb_audio_disconnect().
>>>>
>>>> New structure media_ctl is added to group the new
>>>> fields to support media entity and links. This new
>>>> structure is added to struct snd_usb_substream.
>>>>
>>>> A new entity_notify hook and a new ALSA capture media
>>>> entity are registered from snd_usb_pcm_open() after
>>>> setting up hardware information for the PCM device.
>>>>
>>>> When a new entity is registered, Media Controller API
>>>> interface media_device_register_entity() invokes all
>>>> registered entity_notify hooks for the media device.
>>>> ALSA entity_notify hook parses all the entity list to
>>>> find a link from decoder it ALSA entity. This indicates
>>>> that the bridge driver created a link from decoder to
>>>> ALSA capture entity.
>>>>
>>>> ALSA will attempt to enable the tuner to link the tuner
>>>> to the decoder calling enable_source handler if one is
>>>> provided by the bridge driver prior to starting Media
>>>> pipeline from snd_usb_hw_params(). If enable_source returns
>>>> with tuner busy condition, then snd_usb_hw_params() will fail
>>>> with -EBUSY. Media pipeline is stopped from snd_usb_hw_free().
>>>>
>>>> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
>>>> ---
>>>> Changes since v1: Addressed Takasi Iwai's comments on v1
>>>> - Move config defines to Kconfig and add logic
>>>>   to Makefile to conditionally compile media.c
>>>> - Removed extra includes from media.c
>>>> - Added snd_usb_autosuspend() in error leg
>>>> - Removed debug related code that was missed in v1
>>>>
>>>>  sound/usb/Kconfig        |   7 ++
>>>>  sound/usb/Makefile       |   4 +
>>>>  sound/usb/card.c         |   7 ++
>>>>  sound/usb/card.h         |   1 +
>>>>  sound/usb/media.c        | 190 +++++++++++++++++++++++++++++++++++++++++++++++
>>>>  sound/usb/media.h        |  56 ++++++++++++++
>>>>  sound/usb/pcm.c          |  28 +++++--
>>>>  sound/usb/quirks-table.h |   1 +
>>>>  sound/usb/quirks.c       |   5 ++
>>>>  sound/usb/stream.c       |   2 +
>>>>  sound/usb/usbaudio.h     |   1 +
>>>>  11 files changed, 297 insertions(+), 5 deletions(-)
>>>>  create mode 100644 sound/usb/media.c
>>>>  create mode 100644 sound/usb/media.h
>>>>
>>>> diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig
>>>> index a452ad7..8d5cdab 100644
>>>> --- a/sound/usb/Kconfig
>>>> +++ b/sound/usb/Kconfig
>>>> @@ -15,6 +15,7 @@ config SND_USB_AUDIO
>>>>  	select SND_RAWMIDI
>>>>  	select SND_PCM
>>>>  	select BITREVERSE
>>>> +	select SND_USB_AUDIO_USE_MEDIA_CONTROLLER if MEDIA_CONTROLLER && (MEDIA_SUPPORT || MEDIA_SUPPORT_MODULE)
>>>
>>> This looks wrong as a Kconfig syntax.  "... if MEDIA_CONTROLLER"
>>> should work, I suppose?
>>
>> The conditional select syntax is used in several Kconfig
>> files. You are right about (MEDIA_SUPPORT || MEDIA_SUPPORT_MODULE)
>> Adding checks for that is unnecessary.
> 
> Well, my point was that check should be like:
>    select SND_USB_AUDIO_XXX if MEDIA_CONTROLLER && MEDIA_SUPPORT

Thank you for your patience in helping me through this.

I looked into this more and there is no need to check
for MEDIA_SUPPORT. MEDIA_CONTROLLER defined in

if MEDIA_SUPPORT
endif # MEDIA_SUPPORT

block, checking for MEDIA_SUPPORT is unnecessary.

> 
> There is a revere-select, so this menu itself doesn't play any role.
> 
> If you want this item selectable, drop select from SND_USB_AUDIO but
> just have proper dependencies for this item:

Yes Agreed.

> 
> config SND_USB_AUDIO_USE_MEDIA_CONTROLLER
> 	bool "USB Audio/MIDI driver with Media Controller Support"
> 	depends on SND_USB_AUDIO
> 	depends on MEDIA_CONTROLLER
> 	default y
> 
> (Whether default y or not is a remaining question: usually we keep it 
>  empty (i.e. no), but I put it here since you had it.)
> 
> If you want this auto-selected unconditionally, drop the prompt and
> superfluous dependency (and don't set default y):

Yes I want this auto-selected unconditionally so it won't be
disabled by mistake which would lead to errors because ALSA
will be out of sync with DVB and Analog in the way they try
to get access to media resources.

Yes the below will work.

> 
> config SND_USB_AUDIO
> 	....
> 	select SND_USB_AUDIO_USE_MEDIA_CONTROLLER if MEDIA_CONTROLLER
> 	....
> 
> config SND_USB_AUDIO_USE_MEDIA_CONTROLLER
> 	bool
> 

thanks,
-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

^ permalink raw reply

* Re: Pinnacle PCTV DVB-S2 Stick (461e) - HD Streams with artefacts
From: Andy Furniss @ 2016-01-17 10:27 UTC (permalink / raw)
  To: Rainer Dorsch; +Cc: linux-media
In-Reply-To: <2761448.7zDNhWqk2x@blackbox>

Rainer Dorsch wrote:

>>> Certainly any hint how to solve this issue is welcome.
>>
>> I added a comment on the tvh forum - will paste here as well in
>> case anyone is interested -
>>
>> You could try spinning up you cpu(s) with
>>
>> nice -19 dd if=/dev/urandom of=/dev/null
>
> I added four of these, but if at all then there was only a minor
> impact.

Oh, OK it was worth a try.

>> If you have multiple cores maybe start more than one.
>>
>> I have a couple of DVB-T2 PCTV sticks and got some usb/power
>> save/xhci issues on my h/w.
>>
>> Above would mostly fix. Rather than do that I found that the issue
>> was far less if I disabled USB3 in bios to avoid using the xhci
>> driver.
>
> The cubox-i has no USB 3 port and no bios :-/

Ahh, quite different h/w then. Mine was a quad core baytrail DC board.

>> Of course your issue may be totally different - but it's worth a
>> try.
>
>> Your symptoms do point to ts packet loss - which I know from my
>> experience can be at usb level. There are posts on here from the
>> past where people with PCIE cards also had to do similar.
>
> Sounds reasonable. Can I enable logs which would make these drops
> explicit?

Not that I know of at kernel level. The only way really is to infer loss
from the continuity counters, which TVH already logs.

^ permalink raw reply

* cron job: media_tree daily build: OK
From: Hans Verkuil @ 2016-01-17  4:03 UTC (permalink / raw)
  To: linux-media

This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:		Sun Jan 17 04:00:15 CET 2016
git branch:	test
git hash:	768acf46e1320d6c41ed1b7c4952bab41c1cde79
gcc version:	i686-linux-gcc (GCC) 5.1.0
sparse version:	v0.5.0-51-ga53cea2
smatch version:	v0.5.0-3228-g5cf65ab
host hardware:	x86_64
host os:	4.3.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-exynos: OK
linux-git-arm-mx: OK
linux-git-arm-omap: OK
linux-git-arm-omap1: OK
linux-git-arm-pxa: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.34.7-i686: OK
linux-2.6.35.9-i686: OK
linux-2.6.36.4-i686: OK
linux-2.6.37.6-i686: OK
linux-2.6.38.8-i686: OK
linux-2.6.39.4-i686: OK
linux-3.0.60-i686: OK
linux-3.1.10-i686: OK
linux-3.2.37-i686: OK
linux-3.3.8-i686: OK
linux-3.4.27-i686: OK
linux-3.5.7-i686: OK
linux-3.6.11-i686: OK
linux-3.7.4-i686: OK
linux-3.8-i686: OK
linux-3.9.2-i686: OK
linux-3.10.1-i686: OK
linux-3.11.1-i686: OK
linux-3.12.23-i686: OK
linux-3.13.11-i686: OK
linux-3.14.9-i686: OK
linux-3.15.2-i686: OK
linux-3.16.7-i686: OK
linux-3.17.8-i686: OK
linux-3.18.7-i686: OK
linux-3.19-i686: OK
linux-4.0-i686: OK
linux-4.1.1-i686: OK
linux-4.2-i686: OK
linux-4.3-i686: OK
linux-4.4-i686: OK
linux-2.6.34.7-x86_64: OK
linux-2.6.35.9-x86_64: OK
linux-2.6.36.4-x86_64: OK
linux-2.6.37.6-x86_64: OK
linux-2.6.38.8-x86_64: OK
linux-2.6.39.4-x86_64: OK
linux-3.0.60-x86_64: OK
linux-3.1.10-x86_64: OK
linux-3.2.37-x86_64: OK
linux-3.3.8-x86_64: OK
linux-3.4.27-x86_64: OK
linux-3.5.7-x86_64: OK
linux-3.6.11-x86_64: OK
linux-3.7.4-x86_64: OK
linux-3.8-x86_64: OK
linux-3.9.2-x86_64: OK
linux-3.10.1-x86_64: OK
linux-3.11.1-x86_64: OK
linux-3.12.23-x86_64: OK
linux-3.13.11-x86_64: OK
linux-3.14.9-x86_64: OK
linux-3.15.2-x86_64: OK
linux-3.16.7-x86_64: OK
linux-3.17.8-x86_64: OK
linux-3.18.7-x86_64: OK
linux-3.19-x86_64: OK
linux-4.0-x86_64: OK
linux-4.1.1-x86_64: OK
linux-4.2-x86_64: OK
linux-4.3-x86_64: OK
linux-4.4-x86_64: OK
apps: OK
spec-git: OK
sparse: WARNINGS
smatch: ERRORS

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Sunday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Sunday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/media.html

^ permalink raw reply

* Re: Pinnacle PCTV DVB-S2 Stick (461e) - HD Streams with artefacts
From: Rainer Dorsch @ 2016-01-16 23:43 UTC (permalink / raw)
  To: Andy Furniss; +Cc: linux-media
In-Reply-To: <569ACF59.7090700@gmail.com>

Hi Andy,

thanks for your reply.

On Saturday 16 January 2016 23:16:41 you wrote:
> Rainer Dorsch wrote:
> > Hi,
> > 
> > I have a Pinnacle PCTV DVB-S2 Stick (461e) and did connect it to a
> > cubox-i running openelec.
> > 
> > The known issues on
> > 
> > http://linuxtv.org/wiki/index.php/Pinnacle_PCTV_DVB-S2_Stick_%28461e%29
> > 
> >  seem to be gone (at least I did not hit them).
> > 
> > Instead there seems to be another (new?) issue, that HD streams have
> > artefacts, see
> > 
> > http://forum.kodi.tv/showthread.php?tid=220129
> > 
> > and
> > 
> > https://tvheadend.org/boards/5/topics/19582?r=19584#message-19584
> > 
> > I tried to add that to the wiki, but my registration attempt was
> > rejected.
> > 
> > Can somebody with write access to
> > http://linuxtv.org/wiki/index.php/Pinnacle_PCTV_DVB-S2_Stick_%28461e%29
> > please add the new issue?
> > 
> > Certainly any hint how to solve this issue is welcome.
> 
> I added a comment on the tvh forum - will paste here as well in case
> anyone is interested -
> 
> You could try spinning up you cpu(s) with
> 
> nice -19 dd if=/dev/urandom of=/dev/null

I added four of these, but if at all then there was only a minor impact.

> 
> If you have multiple cores maybe start more than one.
> 
> I have a couple of DVB-T2 PCTV sticks and got some usb/power save/xhci
> issues on my h/w.
> 
> Above would mostly fix. Rather than do that I found that the issue was
> far less if I disabled USB3 in bios to avoid using the xhci driver.

The cubox-i has no USB 3 port and no bios :-/

> Of course your issue may be totally different - but it's worth a try.

> Your symptoms do point to ts packet loss - which I know from my
> experience can be at usb level. There are posts on here from the past
> where people with PCIE cards also had to do similar.

Sounds reasonable. Can I enable logs which would make these drops explicit?

Thanks,
Rainer

-- 
Rainer Dorsch
http://bokomoko.de/

^ permalink raw reply

* Re: Pinnacle PCTV DVB-S2 Stick (461e) - HD Streams with artefacts
From: Andy Furniss @ 2016-01-16 23:16 UTC (permalink / raw)
  To: Rainer Dorsch, linux-media
In-Reply-To: <13463113.ozc26Vzdzi@blackbox>

Rainer Dorsch wrote:
> Hi,
>
> I have a Pinnacle PCTV DVB-S2 Stick (461e) and did connect it to a
> cubox-i running openelec.
>
> The known issues on
>
> http://linuxtv.org/wiki/index.php/Pinnacle_PCTV_DVB-S2_Stick_%28461e%29
>
>  seem to be gone (at least I did not hit them).
>
> Instead there seems to be another (new?) issue, that HD streams have
> artefacts, see
>
> http://forum.kodi.tv/showthread.php?tid=220129
>
> and
>
> https://tvheadend.org/boards/5/topics/19582?r=19584#message-19584
>
> I tried to add that to the wiki, but my registration attempt was
> rejected.
>
> Can somebody with write access to
> http://linuxtv.org/wiki/index.php/Pinnacle_PCTV_DVB-S2_Stick_%28461e%29
> please add the new issue?
>
> Certainly any hint how to solve this issue is welcome.

I added a comment on the tvh forum - will paste here as well in case
anyone is interested -

You could try spinning up you cpu(s) with

nice -19 dd if=/dev/urandom of=/dev/null

If you have multiple cores maybe start more than one.

I have a couple of DVB-T2 PCTV sticks and got some usb/power save/xhci
issues on my h/w.

Above would mostly fix. Rather than do that I found that the issue was
far less if I disabled USB3 in bios to avoid using the xhci driver.

Of course your issue may be totally different - but it's worth a try.

Your symptoms do point to ts packet loss - which I know from my
experience can be at usb level. There are posts on here from the past
where people with PCIE cards also had to do similar.

^ permalink raw reply

* Pinnacle PCTV DVB-S2 Stick (461e) - HD Streams with artefacts
From: Rainer Dorsch @ 2016-01-16 22:03 UTC (permalink / raw)
  To: linux-media

Hi,

I have a Pinnacle PCTV DVB-S2 Stick (461e) and did connect it to a cubox-i 
running openelec.

The known issues on

http://linuxtv.org/wiki/index.php/Pinnacle_PCTV_DVB-S2_Stick_%28461e%29

seem to be gone (at least I did not hit them).

Instead there seems to be another (new?) issue, that HD streams have 
artefacts, see

http://forum.kodi.tv/showthread.php?tid=220129

and

https://tvheadend.org/boards/5/topics/19582?r=19584#message-19584

I tried to add that to the wiki, but my registration attempt was rejected.

Can somebody with write access to 
http://linuxtv.org/wiki/index.php/Pinnacle_PCTV_DVB-S2_Stick_%28461e%29 please 
add the new issue?

Certainly any hint how to solve this issue is welcome.

Thanks
Rainer

-- 
Rainer Dorsch
http://bokomoko.de/

^ permalink raw reply

* Re: [PATCH v2 26/31] sound/usb: Update ALSA driver to use Managed Media Controller API
From: Takashi Iwai @ 2016-01-16  9:06 UTC (permalink / raw)
  To: Shuah Khan
  Cc: hans.verkuil, laurent.pinchart, clemens, sakari.ailus, javier,
	mchehab, alsa-devel, arnd, ricard.wanderlof, labbott,
	chehabrafael, misterpib, prabhakar.csengg, ricardo.ribalda,
	ruchandani.tina, takamichiho, tvboxspy, dominic.sacre, crope,
	julian, pierre-louis.bossart, corbet, joe, johan, dan.carpenter,
	pawel, p.zabel, perex, stefanr, inki.dae, jh1009.sung,
	k.kozlowski, kyungmin.park, m.szyprowski, sw0312.kim, elfring,
	linux-api, linux-kernel, linux-media, linuxbugs, gtmkramer,
	normalperson, daniel
In-Reply-To: <56999103.6000005@osg.samsung.com>

On Sat, 16 Jan 2016 01:38:27 +0100,
Shuah Khan wrote:
> 
> On 01/15/2016 06:38 AM, Takashi Iwai wrote:
> > On Thu, 14 Jan 2016 20:52:28 +0100,
> > Shuah Khan wrote:
> >>
> >> Change ALSA driver to use Managed Media Managed Controller
> >> API to share tuner with DVB and V4L2 drivers that control
> >> AU0828 media device.  Media device is created based on a
> >> newly added field value in the struct snd_usb_audio_quirk.
> >> Using this approach, the media controller API usage can be
> >> added for a specific device. In this patch, Media Controller
> >> API is enabled for AU0828 hw. snd_usb_create_quirk() will
> >> check this new field, if set will create a media device using
> >> media_device_get_devres() interface.
> >>
> >> media_device_get_devres() will allocate a new media device
> >> devres or return an existing one, if it finds one.
> >>
> >> During probe, media usb driver could have created the media
> >> device devres. It will then initialze (if necessary) and
> >> register the media device if it isn't already initialized
> >> and registered. Media device unregister is done from
> >> usb_audio_disconnect().
> >>
> >> During probe, media usb driver could have created the
> >> media device devres. It will then register the media
> >> device if it isn't already registered. Media device
> >> unregister is done from usb_audio_disconnect().
> >>
> >> New structure media_ctl is added to group the new
> >> fields to support media entity and links. This new
> >> structure is added to struct snd_usb_substream.
> >>
> >> A new entity_notify hook and a new ALSA capture media
> >> entity are registered from snd_usb_pcm_open() after
> >> setting up hardware information for the PCM device.
> >>
> >> When a new entity is registered, Media Controller API
> >> interface media_device_register_entity() invokes all
> >> registered entity_notify hooks for the media device.
> >> ALSA entity_notify hook parses all the entity list to
> >> find a link from decoder it ALSA entity. This indicates
> >> that the bridge driver created a link from decoder to
> >> ALSA capture entity.
> >>
> >> ALSA will attempt to enable the tuner to link the tuner
> >> to the decoder calling enable_source handler if one is
> >> provided by the bridge driver prior to starting Media
> >> pipeline from snd_usb_hw_params(). If enable_source returns
> >> with tuner busy condition, then snd_usb_hw_params() will fail
> >> with -EBUSY. Media pipeline is stopped from snd_usb_hw_free().
> >>
> >> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
> >> ---
> >> Changes since v1: Addressed Takasi Iwai's comments on v1
> >> - Move config defines to Kconfig and add logic
> >>   to Makefile to conditionally compile media.c
> >> - Removed extra includes from media.c
> >> - Added snd_usb_autosuspend() in error leg
> >> - Removed debug related code that was missed in v1
> >>
> >>  sound/usb/Kconfig        |   7 ++
> >>  sound/usb/Makefile       |   4 +
> >>  sound/usb/card.c         |   7 ++
> >>  sound/usb/card.h         |   1 +
> >>  sound/usb/media.c        | 190 +++++++++++++++++++++++++++++++++++++++++++++++
> >>  sound/usb/media.h        |  56 ++++++++++++++
> >>  sound/usb/pcm.c          |  28 +++++--
> >>  sound/usb/quirks-table.h |   1 +
> >>  sound/usb/quirks.c       |   5 ++
> >>  sound/usb/stream.c       |   2 +
> >>  sound/usb/usbaudio.h     |   1 +
> >>  11 files changed, 297 insertions(+), 5 deletions(-)
> >>  create mode 100644 sound/usb/media.c
> >>  create mode 100644 sound/usb/media.h
> >>
> >> diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig
> >> index a452ad7..8d5cdab 100644
> >> --- a/sound/usb/Kconfig
> >> +++ b/sound/usb/Kconfig
> >> @@ -15,6 +15,7 @@ config SND_USB_AUDIO
> >>  	select SND_RAWMIDI
> >>  	select SND_PCM
> >>  	select BITREVERSE
> >> +	select SND_USB_AUDIO_USE_MEDIA_CONTROLLER if MEDIA_CONTROLLER && (MEDIA_SUPPORT || MEDIA_SUPPORT_MODULE)
> > 
> > This looks wrong as a Kconfig syntax.  "... if MEDIA_CONTROLLER"
> > should work, I suppose?
> 
> The conditional select syntax is used in several Kconfig
> files. You are right about (MEDIA_SUPPORT || MEDIA_SUPPORT_MODULE)
> Adding checks for that is unnecessary.

Well, my point was that check should be like:
   select SND_USB_AUDIO_XXX if MEDIA_CONTROLLER && MEDIA_SUPPORT

as there is no MEDIA_SUPPORT_MODULE in Kconfig.  It's
MEDIA_SUPPORT=m in Kconfig syntax.  In C code, it's
CONFIG_MEDIA_SUPPORT_MODULE, though.

> > Above all, can MEDIA_CONTROLLER be tristate?  It's currently a bool.
> > If it's a tristate, it causes a problem wrt dependency.  Imagine
> > USB-audio is built-in while MC is a module.
> 
> I don't think MEDIA_CONTROLLER will evebr tristate. I have this
> logic to the following and it works with and without MEDIA_CONTROLLER
> 
> diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig
> index a452ad7..8ccbb35 100644
> --- a/sound/usb/Kconfig
> +++ b/sound/usb/Kconfig
> @@ -15,6 +15,7 @@ config SND_USB_AUDIO
>         select SND_RAWMIDI
>         select SND_PCM
>         select BITREVERSE
> +       select SND_USB_AUDIO_USE_MEDIA_CONTROLLER       if MEDIA_CONTROLLER
>         help
>           Say Y here to include support for USB audio and USB MIDI
>           devices.
> @@ -22,6 +23,11 @@ config SND_USB_AUDIO
>           To compile this driver as a module, choose M here: the module
>           will be called snd-usb-audio.
> 
> +config SND_USB_AUDIO_USE_MEDIA_CONTROLLER
> +       bool "USB Audio/MIDI driver with Media Controller Support"
> +       depends on MEDIA_CONTROLLER
> +       default y
> +
> 
> > 
> >>  	help
> >>  	  Say Y here to include support for USB audio and USB MIDI
> >>  	  devices.
> >> @@ -22,6 +23,12 @@ config SND_USB_AUDIO
> >>  	  To compile this driver as a module, choose M here: the module
> >>  	  will be called snd-usb-audio.
> >>  
> >> +config SND_USB_AUDIO_USE_MEDIA_CONTROLLER
> >> +	bool "USB Audio/MIDI driver with Media Controller Support"
> >> +	depends on SND_USB_AUDIO && MEDIA_CONTROLLER
> >> +	depends on MEDIA_SUPPORT || MEDIA_SUPPORT_MODULE
> 
> See above. I got rid of depends on for SND_USB_AUDIO

There is a revere-select, so this menu itself doesn't play any role.

If you want this item selectable, drop select from SND_USB_AUDIO but
just have proper dependencies for this item:

config SND_USB_AUDIO_USE_MEDIA_CONTROLLER
	bool "USB Audio/MIDI driver with Media Controller Support"
	depends on SND_USB_AUDIO
	depends on MEDIA_CONTROLLER
	default y

(Whether default y or not is a remaining question: usually we keep it 
 empty (i.e. no), but I put it here since you had it.)

If you want this auto-selected unconditionally, drop the prompt and
superfluous dependency (and don't set default y):

config SND_USB_AUDIO
	....
	select SND_USB_AUDIO_USE_MEDIA_CONTROLLER if MEDIA_CONTROLLER
	....

config SND_USB_AUDIO_USE_MEDIA_CONTROLLER
	bool


Takashi

^ permalink raw reply

* cron job: media_tree daily build: OK
From: Hans Verkuil @ 2016-01-16  4:06 UTC (permalink / raw)
  To: linux-media

This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:		Sat Jan 16 04:02:24 CET 2016
git branch:	test
git hash:	768acf46e1320d6c41ed1b7c4952bab41c1cde79
gcc version:	i686-linux-gcc (GCC) 5.1.0
sparse version:	v0.5.0-51-ga53cea2
smatch version:	v0.5.0-3228-g5cf65ab
host hardware:	x86_64
host os:	4.3.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-exynos: OK
linux-git-arm-mx: OK
linux-git-arm-omap: OK
linux-git-arm-omap1: OK
linux-git-arm-pxa: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.34.7-i686: OK
linux-2.6.35.9-i686: OK
linux-2.6.36.4-i686: OK
linux-2.6.37.6-i686: OK
linux-2.6.38.8-i686: OK
linux-2.6.39.4-i686: OK
linux-3.0.60-i686: OK
linux-3.1.10-i686: OK
linux-3.2.37-i686: OK
linux-3.3.8-i686: OK
linux-3.4.27-i686: OK
linux-3.5.7-i686: OK
linux-3.6.11-i686: OK
linux-3.7.4-i686: OK
linux-3.8-i686: OK
linux-3.9.2-i686: OK
linux-3.10.1-i686: OK
linux-3.11.1-i686: OK
linux-3.12.23-i686: OK
linux-3.13.11-i686: OK
linux-3.14.9-i686: OK
linux-3.15.2-i686: OK
linux-3.16.7-i686: OK
linux-3.17.8-i686: OK
linux-3.18.7-i686: OK
linux-3.19-i686: OK
linux-4.0-i686: OK
linux-4.1.1-i686: OK
linux-4.2-i686: OK
linux-4.3-i686: OK
linux-4.4-i686: OK
linux-2.6.34.7-x86_64: OK
linux-2.6.35.9-x86_64: OK
linux-2.6.36.4-x86_64: OK
linux-2.6.37.6-x86_64: OK
linux-2.6.38.8-x86_64: OK
linux-2.6.39.4-x86_64: OK
linux-3.0.60-x86_64: OK
linux-3.1.10-x86_64: OK
linux-3.2.37-x86_64: OK
linux-3.3.8-x86_64: OK
linux-3.4.27-x86_64: OK
linux-3.5.7-x86_64: OK
linux-3.6.11-x86_64: OK
linux-3.7.4-x86_64: OK
linux-3.8-x86_64: OK
linux-3.9.2-x86_64: OK
linux-3.10.1-x86_64: OK
linux-3.11.1-x86_64: OK
linux-3.12.23-x86_64: OK
linux-3.13.11-x86_64: OK
linux-3.14.9-x86_64: OK
linux-3.15.2-x86_64: OK
linux-3.16.7-x86_64: OK
linux-3.17.8-x86_64: OK
linux-3.18.7-x86_64: OK
linux-3.19-x86_64: OK
linux-4.0-x86_64: OK
linux-4.1.1-x86_64: OK
linux-4.2-x86_64: OK
linux-4.3-x86_64: OK
linux-4.4-x86_64: OK
apps: OK
spec-git: OK
sparse: WARNINGS
smatch: ERRORS

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Saturday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Saturday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/media.html

^ permalink raw reply

* Re: [PATCH v2 26/31] sound/usb: Update ALSA driver to use Managed Media Controller API
From: Shuah Khan @ 2016-01-16  0:38 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: hans.verkuil, laurent.pinchart, clemens, sakari.ailus, javier,
	mchehab, alsa-devel, arnd, ricard.wanderlof, labbott,
	chehabrafael, misterpib, prabhakar.csengg, ricardo.ribalda,
	ruchandani.tina, takamichiho, tvboxspy, dominic.sacre, crope,
	julian, pierre-louis.bossart, corbet, joe, johan, dan.carpenter,
	pawel, p.zabel, perex, stefanr, inki.dae, jh1009.sung,
	k.kozlowski, kyungmin.park, m.szyprowski, sw0312.kim, elfring,
	linux-api, linux-kernel, linux-media, linuxbugs, gtmkramer,
	normalperson, daniel, Shuah Khan
In-Reply-To: <s5hmvs7t133.wl-tiwai@suse.de>

On 01/15/2016 06:38 AM, Takashi Iwai wrote:
> On Thu, 14 Jan 2016 20:52:28 +0100,
> Shuah Khan wrote:
>>
>> Change ALSA driver to use Managed Media Managed Controller
>> API to share tuner with DVB and V4L2 drivers that control
>> AU0828 media device.  Media device is created based on a
>> newly added field value in the struct snd_usb_audio_quirk.
>> Using this approach, the media controller API usage can be
>> added for a specific device. In this patch, Media Controller
>> API is enabled for AU0828 hw. snd_usb_create_quirk() will
>> check this new field, if set will create a media device using
>> media_device_get_devres() interface.
>>
>> media_device_get_devres() will allocate a new media device
>> devres or return an existing one, if it finds one.
>>
>> During probe, media usb driver could have created the media
>> device devres. It will then initialze (if necessary) and
>> register the media device if it isn't already initialized
>> and registered. Media device unregister is done from
>> usb_audio_disconnect().
>>
>> During probe, media usb driver could have created the
>> media device devres. It will then register the media
>> device if it isn't already registered. Media device
>> unregister is done from usb_audio_disconnect().
>>
>> New structure media_ctl is added to group the new
>> fields to support media entity and links. This new
>> structure is added to struct snd_usb_substream.
>>
>> A new entity_notify hook and a new ALSA capture media
>> entity are registered from snd_usb_pcm_open() after
>> setting up hardware information for the PCM device.
>>
>> When a new entity is registered, Media Controller API
>> interface media_device_register_entity() invokes all
>> registered entity_notify hooks for the media device.
>> ALSA entity_notify hook parses all the entity list to
>> find a link from decoder it ALSA entity. This indicates
>> that the bridge driver created a link from decoder to
>> ALSA capture entity.
>>
>> ALSA will attempt to enable the tuner to link the tuner
>> to the decoder calling enable_source handler if one is
>> provided by the bridge driver prior to starting Media
>> pipeline from snd_usb_hw_params(). If enable_source returns
>> with tuner busy condition, then snd_usb_hw_params() will fail
>> with -EBUSY. Media pipeline is stopped from snd_usb_hw_free().
>>
>> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
>> ---
>> Changes since v1: Addressed Takasi Iwai's comments on v1
>> - Move config defines to Kconfig and add logic
>>   to Makefile to conditionally compile media.c
>> - Removed extra includes from media.c
>> - Added snd_usb_autosuspend() in error leg
>> - Removed debug related code that was missed in v1
>>
>>  sound/usb/Kconfig        |   7 ++
>>  sound/usb/Makefile       |   4 +
>>  sound/usb/card.c         |   7 ++
>>  sound/usb/card.h         |   1 +
>>  sound/usb/media.c        | 190 +++++++++++++++++++++++++++++++++++++++++++++++
>>  sound/usb/media.h        |  56 ++++++++++++++
>>  sound/usb/pcm.c          |  28 +++++--
>>  sound/usb/quirks-table.h |   1 +
>>  sound/usb/quirks.c       |   5 ++
>>  sound/usb/stream.c       |   2 +
>>  sound/usb/usbaudio.h     |   1 +
>>  11 files changed, 297 insertions(+), 5 deletions(-)
>>  create mode 100644 sound/usb/media.c
>>  create mode 100644 sound/usb/media.h
>>
>> diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig
>> index a452ad7..8d5cdab 100644
>> --- a/sound/usb/Kconfig
>> +++ b/sound/usb/Kconfig
>> @@ -15,6 +15,7 @@ config SND_USB_AUDIO
>>  	select SND_RAWMIDI
>>  	select SND_PCM
>>  	select BITREVERSE
>> +	select SND_USB_AUDIO_USE_MEDIA_CONTROLLER if MEDIA_CONTROLLER && (MEDIA_SUPPORT || MEDIA_SUPPORT_MODULE)
> 
> This looks wrong as a Kconfig syntax.  "... if MEDIA_CONTROLLER"
> should work, I suppose?

The conditional select syntax is used in several Kconfig
files. You are right about (MEDIA_SUPPORT || MEDIA_SUPPORT_MODULE)
Adding checks for that is unnecessary.

> 
> Above all, can MEDIA_CONTROLLER be tristate?  It's currently a bool.
> If it's a tristate, it causes a problem wrt dependency.  Imagine
> USB-audio is built-in while MC is a module.

I don't think MEDIA_CONTROLLER will evebr tristate. I have this
logic to the following and it works with and without MEDIA_CONTROLLER

diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig
index a452ad7..8ccbb35 100644
--- a/sound/usb/Kconfig
+++ b/sound/usb/Kconfig
@@ -15,6 +15,7 @@ config SND_USB_AUDIO
        select SND_RAWMIDI
        select SND_PCM
        select BITREVERSE
+       select SND_USB_AUDIO_USE_MEDIA_CONTROLLER       if MEDIA_CONTROLLER
        help
          Say Y here to include support for USB audio and USB MIDI
          devices.
@@ -22,6 +23,11 @@ config SND_USB_AUDIO
          To compile this driver as a module, choose M here: the module
          will be called snd-usb-audio.

+config SND_USB_AUDIO_USE_MEDIA_CONTROLLER
+       bool "USB Audio/MIDI driver with Media Controller Support"
+       depends on MEDIA_CONTROLLER
+       default y
+

> 
>>  	help
>>  	  Say Y here to include support for USB audio and USB MIDI
>>  	  devices.
>> @@ -22,6 +23,12 @@ config SND_USB_AUDIO
>>  	  To compile this driver as a module, choose M here: the module
>>  	  will be called snd-usb-audio.
>>  
>> +config SND_USB_AUDIO_USE_MEDIA_CONTROLLER
>> +	bool "USB Audio/MIDI driver with Media Controller Support"
>> +	depends on SND_USB_AUDIO && MEDIA_CONTROLLER
>> +	depends on MEDIA_SUPPORT || MEDIA_SUPPORT_MODULE

See above. I got rid of depends on for SND_USB_AUDIO

> 
> Hmm, it's superfluous to have both this depends and the reverse-select
> of the above.  (And again MEDIA_SUPPORT_MODULE looks bogus.)
> 
>> +	default y
>> +
>>  config SND_USB_UA101
>>  	tristate "Edirol UA-101/UA-1000 driver"
>>  	select SND_PCM
>> diff --git a/sound/usb/Makefile b/sound/usb/Makefile
>> index 2d2d122..3113c45 100644
>> --- a/sound/usb/Makefile
>> +++ b/sound/usb/Makefile
>> @@ -15,6 +15,10 @@ snd-usb-audio-objs := 	card.o \
>>  			quirks.o \
>>  			stream.o
>>  
>> +ifeq ($(CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER),y)
>> +  snd-usb-audio-objs += media.o
>> +endif
> 
> A better form is like
> 
> snd-usb-audio-$(CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER) += media.

Got it. Fixed it now.

> 
> 
>> +
>>  snd-usbmidi-lib-objs := midi.o
>>  
>>  # Toplevel Module Dependency
>> diff --git a/sound/usb/card.c b/sound/usb/card.c
>> index 18f5664..1a63851 100644
>> --- a/sound/usb/card.c
>> +++ b/sound/usb/card.c
>> @@ -66,6 +66,7 @@
>>  #include "format.h"
>>  #include "power.h"
>>  #include "stream.h"
>> +#include "media.h"
>>  
>>  MODULE_AUTHOR("Takashi Iwai <tiwai@suse.de>");
>>  MODULE_DESCRIPTION("USB Audio");
>> @@ -621,6 +622,12 @@ static void usb_audio_disconnect(struct usb_interface *intf)
>>  		list_for_each_entry(mixer, &chip->mixer_list, list) {
>>  			snd_usb_mixer_disconnect(mixer);
>>  		}
>> +		/*
>> +		 * Nice to check quirk && quirk->media_device
>> +		 * need some special handlings. Doesn't look like
>> +		 * we have access to quirk here
>> +		*/
>> +		media_device_delete(intf);
>>  	}
>>  
>>  	chip->num_interfaces--;
>> diff --git a/sound/usb/card.h b/sound/usb/card.h
>> index 71778ca..c15a03c 100644
>> --- a/sound/usb/card.h
>> +++ b/sound/usb/card.h
>> @@ -156,6 +156,7 @@ struct snd_usb_substream {
>>  	} dsd_dop;
>>  
>>  	bool trigger_tstamp_pending_update; /* trigger timestamp being updated from initial estimate */
>> +	void *media_ctl;
>>  };
>>  
>>  struct snd_usb_stream {
>> diff --git a/sound/usb/media.c b/sound/usb/media.c
>> new file mode 100644
>> index 0000000..644b6e8
>> --- /dev/null
>> +++ b/sound/usb/media.c
>> @@ -0,0 +1,190 @@
>> +/*
>> + * media.c - Media Controller specific ALSA driver code
>> + *
>> + * Copyright (c) 2015 Shuah Khan <shuahkh@osg.samsung.com>
>> + * Copyright (c) 2015 Samsung Electronics Co., Ltd.
>> + *
>> + * This file is released under the GPLv2.
>> + */
>> +
>> +/*
>> + * This file adds Media Controller support to ALSA driver
>> + * to use the Media Controller API to share tuner with DVB
>> + * and V4L2 drivers that control media device. Media device
>> + * is created based on existing quirks framework. Using this
>> + * approach, the media controller API usage can be added for
>> + * a specific device.
>> +*/
>> +
>> +#include <linux/init.h>
>> +#include <linux/list.h>
>> +#include <linux/mutex.h>
>> +#include <linux/slab.h>
>> +#include <linux/usb.h>
>> +
>> +#include <sound/pcm.h>
>> +
>> +#include "usbaudio.h"
>> +#include "card.h"
>> +#include "media.h"
>> +
>> +int media_device_create(struct snd_usb_audio *chip,
>> +			struct usb_interface *iface)
>> +{
>> +	struct media_device *mdev;
>> +	struct usb_device *usbdev = interface_to_usbdev(iface);
>> +	int ret;
>> +
>> +	mdev = media_device_get_devres(&usbdev->dev);
>> +	if (!mdev)
>> +		return -ENOMEM;
>> +	if (!mdev->dev) {
>> +		/* register media device */
>> +		mdev->dev = &usbdev->dev;
>> +		if (usbdev->product)
>> +			strlcpy(mdev->model, usbdev->product,
>> +				sizeof(mdev->model));
>> +		if (usbdev->serial)
>> +			strlcpy(mdev->serial, usbdev->serial,
>> +				sizeof(mdev->serial));
>> +		strcpy(mdev->bus_info, usbdev->devpath);
>> +		mdev->hw_revision = le16_to_cpu(usbdev->descriptor.bcdDevice);
>> +		ret = media_device_init(mdev);
>> +		if (ret) {
>> +			dev_err(&usbdev->dev,
>> +				"Couldn't create a media device. Error: %d\n",
>> +				ret);
>> +			return ret;
>> +		}
>> +	}
>> +	if (!media_devnode_is_registered(&mdev->devnode)) {
>> +		ret = media_device_register(mdev);
>> +		if (ret) {
>> +			dev_err(&usbdev->dev,
>> +				"Couldn't register media device. Error: %d\n",
>> +				ret);
>> +			return ret;
>> +		}
>> +	}
>> +	return 0;
>> +}
>> +
>> +void media_device_delete(struct usb_interface *iface)
>> +{
>> +	struct media_device *mdev;
>> +	struct usb_device *usbdev = interface_to_usbdev(iface);
>> +
>> +	mdev = media_device_find_devres(&usbdev->dev);
>> +	if (mdev && media_devnode_is_registered(&mdev->devnode))
>> +		media_device_unregister(mdev);
>> +}
>> +
>> +static int media_enable_source(struct media_ctl *mctl)
>> +{
>> +	if (mctl && mctl->media_dev->enable_source)
>> +		return mctl->media_dev->enable_source(&mctl->media_entity,
>> +						      &mctl->media_pipe);
>> +	return 0;
>> +}
>> +
>> +static void media_disable_source(struct media_ctl *mctl)
>> +{
>> +	if (mctl && mctl->media_dev->disable_source)
>> +		mctl->media_dev->disable_source(&mctl->media_entity);
>> +}
>> +
>> +int media_stream_init(struct snd_usb_substream *subs, struct snd_pcm *pcm,
>> +			int stream)
>> +{
>> +	struct media_device *mdev;
>> +	struct media_ctl *mctl;
>> +	struct device *pcm_dev = &pcm->streams[stream].dev;
>> +	u32 intf_type;
>> +	int ret = 0;
>> +
>> +	mdev = media_device_find_devres(&subs->dev->dev);
>> +	if (!mdev)
>> +		return -ENODEV;
>> +
>> +	if (subs->media_ctl)
>> +		return 0;
>> +
>> +	/* allocate media_ctl */
>> +	mctl = kzalloc(sizeof(*mctl), GFP_KERNEL);
>> +	if (!mctl)
>> +		return -ENOMEM;
>> +
>> +	mctl->media_dev = mdev;
>> +	if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
>> +		intf_type = MEDIA_INTF_T_ALSA_PCM_PLAYBACK;
>> +		mctl->media_entity.function = MEDIA_ENT_F_AUDIO_PLAYBACK;
>> +	} else {
>> +		intf_type = MEDIA_INTF_T_ALSA_PCM_CAPTURE;
>> +		mctl->media_entity.function = MEDIA_ENT_F_AUDIO_CAPTURE;
>> +	}
>> +	mctl->media_entity.name = pcm->name;
>> +	mctl->media_entity.info.dev.major = MAJOR(pcm_dev->devt);
>> +	mctl->media_entity.info.dev.minor = MINOR(pcm_dev->devt);
>> +	mctl->media_pad.flags = MEDIA_PAD_FL_SINK;
>> +	media_entity_pads_init(&mctl->media_entity, 1, &mctl->media_pad);
>> +	ret =  media_device_register_entity(mctl->media_dev,
>> +					    &mctl->media_entity);
>> +	if (ret) {
>> +		kfree(mctl);
>> +		return ret;
>> +	}
>> +	mctl->intf_devnode = media_devnode_create(mdev, intf_type, 0,
>> +						  MAJOR(pcm_dev->devt),
>> +						  MINOR(pcm_dev->devt));
>> +	if (!mctl->intf_devnode) {
>> +		media_device_unregister_entity(&mctl->media_entity);
>> +		kfree(mctl);
>> +		return -ENOMEM;
>> +	}
>> +	mctl->intf_link = media_create_intf_link(&mctl->media_entity,
>> +						 &mctl->intf_devnode->intf,
>> +						 MEDIA_LNK_FL_ENABLED);
>> +	if (!mctl->intf_link) {
>> +		media_devnode_remove(mctl->intf_devnode);
>> +		media_device_unregister_entity(&mctl->media_entity);
>> +		kfree(mctl);
>> +		return -ENOMEM;
>> +	}
>> +	subs->media_ctl = (void *) mctl;
> 
> The cast to a void pointer is superfluous.

Right. Fixed it and couple of others like this
one I found.

Will send v3 shortly.

thanks,
-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

^ permalink raw reply related

* RE: V4L2 encoder APIs
From: Sebastien LEDUC @ 2016-01-15 17:33 UTC (permalink / raw)
  To: Guennadi Liakhovetski; +Cc: linux-media@vger.kernel.org
In-Reply-To: <Pine.LNX.4.64.1601132156280.13265@axis700.grange>

Thanks for your answer

> I think this should be handled in the same way as the output direction. We
> are currently discussing this with several V4L developers. For output we have
> to capture different data types to different buffers, running multiplexed on
> the bus, e.g. over CSI-2. Using the MPLANE API would be one option, but you
> don't want to define a new pixel format for each combination of each
> standard pixel format with each accompanying data type, be it metadata or
> anything else. So, you would have to add support for per-plane format,
> which would contradict the current MPLANE API concept.

Dont know what are the other use cases you are discussing
But in our case there is only one combination:
The first plane should contain the header data, and the second one should contain the slice data

At the end the compressed frame is just the concatenation of the first buffer (header) and the second one (slice data)

So there is no reason to have multiple combinations in the future

> 
> Therefore we're currently considering a different option of transferring
> different buffer types via different buffer queues. Initially we thought about
> simply using multiple video device nodes. That has a disadvantage when the
> number of those streams is variable and potentially large. So, another option
> is to add support for multiple buffer queues per video node. Those buffer
> queues would then have to get some form of identification, perhaps a
> stream ID. That stream ID would also be used to associate those streams to
> links between subdevice pads. That's all still very raw... Quite a bit of design
> and implementation work ahead.
> 


In our case using separate buffer queues does not look very appropriate, because there is a strong ordering constraint, and we always want to get a header and slice data together

So my current feeling is really that MPLANE is the best fit for our need

Regards
Sebastien

^ permalink raw reply

* Re: [PATCH] v4l: add V4L2_CID_MPEG_VIDEO_FORCE_FRAME_TYPE.
From: Nicolas Dufresne @ 2016-01-15 17:21 UTC (permalink / raw)
  To: Kamil Debski, 'Wu-Cheng Li', pawel, mchehab, hverkuil,
	crope, standby24x7, ricardo.ribalda, ao2, bparrot,
	'Andrzej Hajda'
  Cc: linux-media, linux-kernel, linux-api
In-Reply-To: <000301d14fb6$5cdd6370$16982a50$@samsung.com>

[-- Attachment #1: Type: text/plain, Size: 1893 bytes --]

Le vendredi 15 janvier 2016 à 18:01 +0100, Kamil Debski a écrit :
> I think we had a discussion about this long, long time ago. Should it
> be
> deterministic which frame Is encoded as a key frame? Should it be the
> next queued frame, or the next processed frame? How to achieve this?
> I vaguely remember that we discussed per buffer controls on the
> mailing
> list, but I am not sure where the discussion went from there.

In RTP use cases (and most streaming cases), all we care is that we
have a key-frame produced somewhere nearby after the request. In those
cases we use P frame only stream to reduce the bandwidth and only
request a key frame for recovery. This I believe that most common case.

Many decoders though also offer what they called an "automatic" key
frame mode. In the case of x264, there is also hints you can give to
the encoder about what type of video (film, anime, etc.) so it can
optimize all this. I believe this is very advance and there is no
particular need for it.

> 
> > 
> > - A way to trigger a key frame to be produce
> > - A way to produce a I-Frame only stream
> 
> This control can be used to do this:
> - V4L2_CID_MPEG_VIDEO_GOP_SIZE (It is not well documented as I can
> see ;) )
>         + If set to 0 the encoder produces a stream with P only
> frames
>         + if set to 1 the encoder produces a stream with I only
> frames
>         + other values indicate the GOP size (I-frame interval)
> 
> > - A way to set the key-frame distance (in frames) even though this
> could
> > possibly be emulated using the trigger.
> 
> As described above V4L2_CID_MPEG_VIDEO_GOP_SIZE can be used to
> achieve this.

Great, my memory failed on me, should have checked. This is exactly
what I was thinking. So Wu-Cheng Li patch is really what we need in the
immediate.

regards,
Nicolas

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* Re: [PATCH] af9035: add support for 2nd tuner of MSI DigiVox Diversity
From: Stefan Pöschel @ 2016-01-15 17:13 UTC (permalink / raw)
  To: Linux Media Mailing List
In-Reply-To: <568AE147.6070908@gmx.de>

Hi,

I just wanted to ask if there is any further information needed regarding my patch.

Best regards,
	Stefan

Am 04.01.2016 um 22:16 schrieb Stefan Pöschel:
> PIP tested with VLC. Diversity tested with the Windows driver.
> 
> Signed-off-by: Stefan Pöschel <basic.master@gmx.de>
> ---
>  drivers/media/usb/dvb-usb-v2/af9035.c | 4 ++--
>  drivers/media/usb/dvb-usb-v2/af9035.h | 3 ++-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c b/drivers/media/usb/dvb-usb-v2/af9035.c
> index 6e02a15..b3c09fe 100644
> --- a/drivers/media/usb/dvb-usb-v2/af9035.c
> +++ b/drivers/media/usb/dvb-usb-v2/af9035.c
> @@ -684,7 +684,7 @@ static int af9035_download_firmware(struct dvb_usb_device *d,
>  	if (ret < 0)
>  		goto err;
> 
> -	if (tmp =1 || tmp == 3) {
> +	if (tmp =1 || tmp == 3 || tmp == 5) {
>  		/* configure gpioh1, reset & power slave demod */
>  		ret =f9035_wr_reg_mask(d, 0x00d8b0, 0x01, 0x01);
>  		if (ret < 0)
> @@ -823,7 +823,7 @@ static int af9035_read_config(struct dvb_usb_device *d)
>  	if (ret < 0)
>  		goto err;
> 
> -	if (tmp =1 || tmp == 3)
> +	if (tmp =1 || tmp == 3 || tmp == 5)
>  		state->dual_mode =rue;
> 
>  	dev_dbg(&d->udev->dev, "%s: ts mode= dual mode=%d\n", __func__,
> diff --git a/drivers/media/usb/dvb-usb-v2/af9035.h b/drivers/media/usb/dvb-usb-v2/af9035.h
> index 416a97f..df22001 100644
> --- a/drivers/media/usb/dvb-usb-v2/af9035.h
> +++ b/drivers/media/usb/dvb-usb-v2/af9035.h
> @@ -112,9 +112,10 @@ static const u32 clock_lut_it9135[] =
>   * 0  TS
>   * 1  DCA + PIP
>   * 3  PIP
> + * 5  DCA + PIP
>   * n  DCA
>   *
> - * Values 0 and 3 are seen to this day. 0 for single TS and 3 for dual TS.
> + * Values 0, 3 and 5 are seen to this day. 0 for single TS and 3/5 for dual TS.
>   */
> 
>  #define EEPROM_BASE_AF9035        0x42fd
> 

^ permalink raw reply

* RE: [PATCH] v4l: add V4L2_CID_MPEG_VIDEO_FORCE_FRAME_TYPE.
From: Kamil Debski @ 2016-01-15 17:01 UTC (permalink / raw)
  To: 'Nicolas Dufresne', 'Wu-Cheng Li', pawel, mchehab,
	hverkuil, crope, standby24x7, ricardo.ribalda, ao2, bparrot,
	'Andrzej Hajda'
  Cc: linux-media, linux-kernel, linux-api
In-Reply-To: <1452798133.3306.3.camel@collabora.com>

Hi,

> From: Nicolas Dufresne [mailto:nicolas.dufresne@collabora.com]
> Sent: Thursday, January 14, 2016 8:02 PM
> To: Kamil Debski; 'Wu-Cheng Li'; pawel@osciak.com;
> mchehab@osg.samsung.com; hverkuil@xs4all.nl; crope@iki.fi;
> standby24x7@gmail.com; ricardo.ribalda@gmail.com; ao2@ao2.it;
> bparrot@ti.com; 'Andrzej Hajda'
> Cc: linux-media@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> api@vger.kernel.org
> Subject: Re: [PATCH] v4l: add
> V4L2_CID_MPEG_VIDEO_FORCE_FRAME_TYPE.
> 
> Le jeudi 14 janvier 2016 à 18:21 +0100, Kamil Debski a écrit :
> > I had a look into the documentation of MFC. It is possible to force
> > two types of a frame to be coded.
> > The one is a keyframe, the other is a not coded frame. As I understand
> > this is a type of frame that means that image did not change from
> > previous frame. I am sure I seen it in an MPEG4 stream in a movie
> > trailer. The initial board with the age rating is displayed for a
> > couple of seconds and remains static. Thus there is one I-frame and a
> > number of non-coded frames.
> >
> > That is the reason why the control was implemented in MFC as a menu
> > and not a button. Thus the question remains - is it better to leave it
> > as a menu, or should there be two (maybe more in the future) buttons?
> 
> Then I believe we need both. Because with the menu, setting I-Frame, I
> would expect to only receive keyframes from now-on. While the useful
> feature we are looking for here, is to get the next buffer (or nearby) to be a
> keyframe. It's the difference between creating an I-Frame only stream and
> requesting a key-frame manually for recovery (RTP use case).
> In this end, we should probably take that time to review the features we
> have as we need:

I think we had a discussion about this long, long time ago. Should it be
deterministic which frame Is encoded as a key frame? Should it be the
next queued frame, or the next processed frame? How to achieve this?
I vaguely remember that we discussed per buffer controls on the mailing
list, but I am not sure where the discussion went from there.

> 
> - A way to trigger a key frame to be produce
> - A way to produce a I-Frame only stream

This control can be used to do this:
- V4L2_CID_MPEG_VIDEO_GOP_SIZE (It is not well documented as I can see ;) )
	+ If set to 0 the encoder produces a stream with P only frames
	+ if set to 1 the encoder produces a stream with I only frames
	+ other values indicate the GOP size (I-frame interval)

> - A way to set the key-frame distance (in frames) even though this could
> possibly be emulated using the trigger.

As described above V4L2_CID_MPEG_VIDEO_GOP_SIZE can be used to achieve this.

> 
> cheers,
>Nicolas
 
Best wishes,
-- 
Kamil Debski
Samsung R&D Institute Poland


^ permalink raw reply

* Re: [PATCH v2 26/31] sound/usb: Update ALSA driver to use Managed Media Controller API
From: Takashi Iwai @ 2016-01-15 13:38 UTC (permalink / raw)
  To: Shuah Khan
  Cc: hans.verkuil, laurent.pinchart, clemens, sakari.ailus, javier,
	mchehab, alsa-devel, arnd, ricard.wanderlof, labbott,
	chehabrafael, misterpib, prabhakar.csengg, ricardo.ribalda,
	ruchandani.tina, takamichiho, tvboxspy, dominic.sacre, crope,
	julian, pierre-louis.bossart, corbet, joe, johan, dan.carpenter,
	pawel, p.zabel, perex, stefanr, inki.dae, jh1009.sung,
	k.kozlowski, kyungmin.park, m.szyprowski, sw0312.kim, elfring,
	linux-api, linux-kernel, linux-media, linuxbugs, gtmkramer,
	normalperson, daniel
In-Reply-To: <fd72f02797d595fc1c53b16fccec4d204430995e.1452800273.git.shuahkh@osg.samsung.com>

On Thu, 14 Jan 2016 20:52:28 +0100,
Shuah Khan wrote:
> 
> Change ALSA driver to use Managed Media Managed Controller
> API to share tuner with DVB and V4L2 drivers that control
> AU0828 media device.  Media device is created based on a
> newly added field value in the struct snd_usb_audio_quirk.
> Using this approach, the media controller API usage can be
> added for a specific device. In this patch, Media Controller
> API is enabled for AU0828 hw. snd_usb_create_quirk() will
> check this new field, if set will create a media device using
> media_device_get_devres() interface.
> 
> media_device_get_devres() will allocate a new media device
> devres or return an existing one, if it finds one.
> 
> During probe, media usb driver could have created the media
> device devres. It will then initialze (if necessary) and
> register the media device if it isn't already initialized
> and registered. Media device unregister is done from
> usb_audio_disconnect().
> 
> During probe, media usb driver could have created the
> media device devres. It will then register the media
> device if it isn't already registered. Media device
> unregister is done from usb_audio_disconnect().
> 
> New structure media_ctl is added to group the new
> fields to support media entity and links. This new
> structure is added to struct snd_usb_substream.
> 
> A new entity_notify hook and a new ALSA capture media
> entity are registered from snd_usb_pcm_open() after
> setting up hardware information for the PCM device.
> 
> When a new entity is registered, Media Controller API
> interface media_device_register_entity() invokes all
> registered entity_notify hooks for the media device.
> ALSA entity_notify hook parses all the entity list to
> find a link from decoder it ALSA entity. This indicates
> that the bridge driver created a link from decoder to
> ALSA capture entity.
> 
> ALSA will attempt to enable the tuner to link the tuner
> to the decoder calling enable_source handler if one is
> provided by the bridge driver prior to starting Media
> pipeline from snd_usb_hw_params(). If enable_source returns
> with tuner busy condition, then snd_usb_hw_params() will fail
> with -EBUSY. Media pipeline is stopped from snd_usb_hw_free().
> 
> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
> ---
> Changes since v1: Addressed Takasi Iwai's comments on v1
> - Move config defines to Kconfig and add logic
>   to Makefile to conditionally compile media.c
> - Removed extra includes from media.c
> - Added snd_usb_autosuspend() in error leg
> - Removed debug related code that was missed in v1
> 
>  sound/usb/Kconfig        |   7 ++
>  sound/usb/Makefile       |   4 +
>  sound/usb/card.c         |   7 ++
>  sound/usb/card.h         |   1 +
>  sound/usb/media.c        | 190 +++++++++++++++++++++++++++++++++++++++++++++++
>  sound/usb/media.h        |  56 ++++++++++++++
>  sound/usb/pcm.c          |  28 +++++--
>  sound/usb/quirks-table.h |   1 +
>  sound/usb/quirks.c       |   5 ++
>  sound/usb/stream.c       |   2 +
>  sound/usb/usbaudio.h     |   1 +
>  11 files changed, 297 insertions(+), 5 deletions(-)
>  create mode 100644 sound/usb/media.c
>  create mode 100644 sound/usb/media.h
> 
> diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig
> index a452ad7..8d5cdab 100644
> --- a/sound/usb/Kconfig
> +++ b/sound/usb/Kconfig
> @@ -15,6 +15,7 @@ config SND_USB_AUDIO
>  	select SND_RAWMIDI
>  	select SND_PCM
>  	select BITREVERSE
> +	select SND_USB_AUDIO_USE_MEDIA_CONTROLLER if MEDIA_CONTROLLER && (MEDIA_SUPPORT || MEDIA_SUPPORT_MODULE)

This looks wrong as a Kconfig syntax.  "... if MEDIA_CONTROLLER"
should work, I suppose?

Above all, can MEDIA_CONTROLLER be tristate?  It's currently a bool.
If it's a tristate, it causes a problem wrt dependency.  Imagine
USB-audio is built-in while MC is a module.

>  	help
>  	  Say Y here to include support for USB audio and USB MIDI
>  	  devices.
> @@ -22,6 +23,12 @@ config SND_USB_AUDIO
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called snd-usb-audio.
>  
> +config SND_USB_AUDIO_USE_MEDIA_CONTROLLER
> +	bool "USB Audio/MIDI driver with Media Controller Support"
> +	depends on SND_USB_AUDIO && MEDIA_CONTROLLER
> +	depends on MEDIA_SUPPORT || MEDIA_SUPPORT_MODULE

Hmm, it's superfluous to have both this depends and the reverse-select
of the above.  (And again MEDIA_SUPPORT_MODULE looks bogus.)

> +	default y
> +
>  config SND_USB_UA101
>  	tristate "Edirol UA-101/UA-1000 driver"
>  	select SND_PCM
> diff --git a/sound/usb/Makefile b/sound/usb/Makefile
> index 2d2d122..3113c45 100644
> --- a/sound/usb/Makefile
> +++ b/sound/usb/Makefile
> @@ -15,6 +15,10 @@ snd-usb-audio-objs := 	card.o \
>  			quirks.o \
>  			stream.o
>  
> +ifeq ($(CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER),y)
> +  snd-usb-audio-objs += media.o
> +endif

A better form is like

snd-usb-audio-$(CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER) += media.


> +
>  snd-usbmidi-lib-objs := midi.o
>  
>  # Toplevel Module Dependency
> diff --git a/sound/usb/card.c b/sound/usb/card.c
> index 18f5664..1a63851 100644
> --- a/sound/usb/card.c
> +++ b/sound/usb/card.c
> @@ -66,6 +66,7 @@
>  #include "format.h"
>  #include "power.h"
>  #include "stream.h"
> +#include "media.h"
>  
>  MODULE_AUTHOR("Takashi Iwai <tiwai@suse.de>");
>  MODULE_DESCRIPTION("USB Audio");
> @@ -621,6 +622,12 @@ static void usb_audio_disconnect(struct usb_interface *intf)
>  		list_for_each_entry(mixer, &chip->mixer_list, list) {
>  			snd_usb_mixer_disconnect(mixer);
>  		}
> +		/*
> +		 * Nice to check quirk && quirk->media_device
> +		 * need some special handlings. Doesn't look like
> +		 * we have access to quirk here
> +		*/
> +		media_device_delete(intf);
>  	}
>  
>  	chip->num_interfaces--;
> diff --git a/sound/usb/card.h b/sound/usb/card.h
> index 71778ca..c15a03c 100644
> --- a/sound/usb/card.h
> +++ b/sound/usb/card.h
> @@ -156,6 +156,7 @@ struct snd_usb_substream {
>  	} dsd_dop;
>  
>  	bool trigger_tstamp_pending_update; /* trigger timestamp being updated from initial estimate */
> +	void *media_ctl;
>  };
>  
>  struct snd_usb_stream {
> diff --git a/sound/usb/media.c b/sound/usb/media.c
> new file mode 100644
> index 0000000..644b6e8
> --- /dev/null
> +++ b/sound/usb/media.c
> @@ -0,0 +1,190 @@
> +/*
> + * media.c - Media Controller specific ALSA driver code
> + *
> + * Copyright (c) 2015 Shuah Khan <shuahkh@osg.samsung.com>
> + * Copyright (c) 2015 Samsung Electronics Co., Ltd.
> + *
> + * This file is released under the GPLv2.
> + */
> +
> +/*
> + * This file adds Media Controller support to ALSA driver
> + * to use the Media Controller API to share tuner with DVB
> + * and V4L2 drivers that control media device. Media device
> + * is created based on existing quirks framework. Using this
> + * approach, the media controller API usage can be added for
> + * a specific device.
> +*/
> +
> +#include <linux/init.h>
> +#include <linux/list.h>
> +#include <linux/mutex.h>
> +#include <linux/slab.h>
> +#include <linux/usb.h>
> +
> +#include <sound/pcm.h>
> +
> +#include "usbaudio.h"
> +#include "card.h"
> +#include "media.h"
> +
> +int media_device_create(struct snd_usb_audio *chip,
> +			struct usb_interface *iface)
> +{
> +	struct media_device *mdev;
> +	struct usb_device *usbdev = interface_to_usbdev(iface);
> +	int ret;
> +
> +	mdev = media_device_get_devres(&usbdev->dev);
> +	if (!mdev)
> +		return -ENOMEM;
> +	if (!mdev->dev) {
> +		/* register media device */
> +		mdev->dev = &usbdev->dev;
> +		if (usbdev->product)
> +			strlcpy(mdev->model, usbdev->product,
> +				sizeof(mdev->model));
> +		if (usbdev->serial)
> +			strlcpy(mdev->serial, usbdev->serial,
> +				sizeof(mdev->serial));
> +		strcpy(mdev->bus_info, usbdev->devpath);
> +		mdev->hw_revision = le16_to_cpu(usbdev->descriptor.bcdDevice);
> +		ret = media_device_init(mdev);
> +		if (ret) {
> +			dev_err(&usbdev->dev,
> +				"Couldn't create a media device. Error: %d\n",
> +				ret);
> +			return ret;
> +		}
> +	}
> +	if (!media_devnode_is_registered(&mdev->devnode)) {
> +		ret = media_device_register(mdev);
> +		if (ret) {
> +			dev_err(&usbdev->dev,
> +				"Couldn't register media device. Error: %d\n",
> +				ret);
> +			return ret;
> +		}
> +	}
> +	return 0;
> +}
> +
> +void media_device_delete(struct usb_interface *iface)
> +{
> +	struct media_device *mdev;
> +	struct usb_device *usbdev = interface_to_usbdev(iface);
> +
> +	mdev = media_device_find_devres(&usbdev->dev);
> +	if (mdev && media_devnode_is_registered(&mdev->devnode))
> +		media_device_unregister(mdev);
> +}
> +
> +static int media_enable_source(struct media_ctl *mctl)
> +{
> +	if (mctl && mctl->media_dev->enable_source)
> +		return mctl->media_dev->enable_source(&mctl->media_entity,
> +						      &mctl->media_pipe);
> +	return 0;
> +}
> +
> +static void media_disable_source(struct media_ctl *mctl)
> +{
> +	if (mctl && mctl->media_dev->disable_source)
> +		mctl->media_dev->disable_source(&mctl->media_entity);
> +}
> +
> +int media_stream_init(struct snd_usb_substream *subs, struct snd_pcm *pcm,
> +			int stream)
> +{
> +	struct media_device *mdev;
> +	struct media_ctl *mctl;
> +	struct device *pcm_dev = &pcm->streams[stream].dev;
> +	u32 intf_type;
> +	int ret = 0;
> +
> +	mdev = media_device_find_devres(&subs->dev->dev);
> +	if (!mdev)
> +		return -ENODEV;
> +
> +	if (subs->media_ctl)
> +		return 0;
> +
> +	/* allocate media_ctl */
> +	mctl = kzalloc(sizeof(*mctl), GFP_KERNEL);
> +	if (!mctl)
> +		return -ENOMEM;
> +
> +	mctl->media_dev = mdev;
> +	if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
> +		intf_type = MEDIA_INTF_T_ALSA_PCM_PLAYBACK;
> +		mctl->media_entity.function = MEDIA_ENT_F_AUDIO_PLAYBACK;
> +	} else {
> +		intf_type = MEDIA_INTF_T_ALSA_PCM_CAPTURE;
> +		mctl->media_entity.function = MEDIA_ENT_F_AUDIO_CAPTURE;
> +	}
> +	mctl->media_entity.name = pcm->name;
> +	mctl->media_entity.info.dev.major = MAJOR(pcm_dev->devt);
> +	mctl->media_entity.info.dev.minor = MINOR(pcm_dev->devt);
> +	mctl->media_pad.flags = MEDIA_PAD_FL_SINK;
> +	media_entity_pads_init(&mctl->media_entity, 1, &mctl->media_pad);
> +	ret =  media_device_register_entity(mctl->media_dev,
> +					    &mctl->media_entity);
> +	if (ret) {
> +		kfree(mctl);
> +		return ret;
> +	}
> +	mctl->intf_devnode = media_devnode_create(mdev, intf_type, 0,
> +						  MAJOR(pcm_dev->devt),
> +						  MINOR(pcm_dev->devt));
> +	if (!mctl->intf_devnode) {
> +		media_device_unregister_entity(&mctl->media_entity);
> +		kfree(mctl);
> +		return -ENOMEM;
> +	}
> +	mctl->intf_link = media_create_intf_link(&mctl->media_entity,
> +						 &mctl->intf_devnode->intf,
> +						 MEDIA_LNK_FL_ENABLED);
> +	if (!mctl->intf_link) {
> +		media_devnode_remove(mctl->intf_devnode);
> +		media_device_unregister_entity(&mctl->media_entity);
> +		kfree(mctl);
> +		return -ENOMEM;
> +	}
> +	subs->media_ctl = (void *) mctl;

The cast to a void pointer is superfluous.


> +	return 0;
> +}
> +
> +void media_stream_delete(struct snd_usb_substream *subs)
> +{
> +	struct media_ctl *mctl = (struct media_ctl *) subs->media_ctl;

The cast from a void pointer is superfluous, too.

> +
> +	if (mctl && mctl->media_dev) {
> +		struct media_device *mdev;
> +
> +		mdev = media_device_find_devres(&subs->dev->dev);
> +		if (mdev) {
> +			media_devnode_remove(mctl->intf_devnode);
> +			media_device_unregister_entity(&mctl->media_entity);
> +			media_entity_cleanup(&mctl->media_entity);
> +		}
> +		kfree(mctl);
> +		subs->media_ctl = NULL;
> +	}
> +}
> +
> +int media_start_pipeline(struct snd_usb_substream *subs)
> +{
> +	struct media_ctl *mctl = (struct media_ctl *) subs->media_ctl;
> +
> +	if (mctl)
> +		return media_enable_source(mctl);
> +	return 0;
> +}
> +
> +void media_stop_pipeline(struct snd_usb_substream *subs)
> +{
> +	struct media_ctl *mctl = (struct media_ctl *) subs->media_ctl;
> +
> +	if (mctl)
> +		media_disable_source(mctl);
> +}
> diff --git a/sound/usb/media.h b/sound/usb/media.h
> new file mode 100644
> index 0000000..00b77e6
> --- /dev/null
> +++ b/sound/usb/media.h
> @@ -0,0 +1,56 @@
> +/*
> + * media.h - Media Controller specific ALSA driver code
> + *
> + * Copyright (c) 2015 Shuah Khan <shuahkh@osg.samsung.com>
> + * Copyright (c) 2015 Samsung Electronics Co., Ltd.
> + *
> + * This file is released under the GPLv2.
> + */
> +
> +/*
> + * This file adds Media Controller support to ALSA driver
> + * to use the Media Controller API to share tuner with DVB
> + * and V4L2 drivers that control media device. Media device
> + * is created based on existing quirks framework. Using this
> + * approach, the media controller API usage can be added for
> + * a specific device.
> +*/
> +#ifndef __MEDIA_H
> +
> +#if defined(CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER) || \
> +	defined(CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER_MODULE)

The kconfig is a bool.


thanks,

Takashi

> +#include <media/media-device.h>
> +#include <media/media-entity.h>
> +
> +struct media_ctl {
> +	struct media_device *media_dev;
> +	struct media_entity media_entity;
> +	struct media_intf_devnode *intf_devnode;
> +	struct media_link *intf_link;
> +	struct media_pad media_pad;
> +	struct media_pipeline media_pipe;
> +};
> +
> +int media_device_create(struct snd_usb_audio *chip,
> +			struct usb_interface *iface);
> +void media_device_delete(struct usb_interface *iface);
> +int media_stream_init(struct snd_usb_substream *subs, struct snd_pcm *pcm,
> +			int stream);
> +void media_stream_delete(struct snd_usb_substream *subs);
> +int media_start_pipeline(struct snd_usb_substream *subs);
> +void media_stop_pipeline(struct snd_usb_substream *subs);
> +#else
> +static inline int media_device_create(struct snd_usb_audio *chip,
> +				      struct usb_interface *iface)
> +						{ return 0; }
> +static inline void media_device_delete(struct usb_interface *iface) { }
> +static inline int media_stream_init(struct snd_usb_substream *subs,
> +					struct snd_pcm *pcm, int stream)
> +						{ return 0; }
> +static inline void media_stream_delete(struct snd_usb_substream *subs) { }
> +static inline int media_start_pipeline(struct snd_usb_substream *subs)
> +					{ return 0; }
> +static inline void media_stop_pipeline(struct snd_usb_substream *subs) { }
> +#endif
> +#endif /* __MEDIA_H */
> diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
> index 9245f52..c3b8486 100644
> --- a/sound/usb/pcm.c
> +++ b/sound/usb/pcm.c
> @@ -35,6 +35,7 @@
>  #include "pcm.h"
>  #include "clock.h"
>  #include "power.h"
> +#include "media.h"
>  
>  #define SUBSTREAM_FLAG_DATA_EP_STARTED	0
>  #define SUBSTREAM_FLAG_SYNC_EP_STARTED	1
> @@ -715,10 +716,14 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream,
>  	struct audioformat *fmt;
>  	int ret;
>  
> +	ret = media_start_pipeline(subs);
> +	if (ret)
> +		return ret;
> +
>  	ret = snd_pcm_lib_alloc_vmalloc_buffer(substream,
>  					       params_buffer_bytes(hw_params));
>  	if (ret < 0)
> -		return ret;
> +		goto err_ret;
>  
>  	subs->pcm_format = params_format(hw_params);
>  	subs->period_bytes = params_period_bytes(hw_params);
> @@ -732,22 +737,27 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream,
>  		dev_dbg(&subs->dev->dev,
>  			"cannot set format: format = %#x, rate = %d, channels = %d\n",
>  			   subs->pcm_format, subs->cur_rate, subs->channels);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto err_ret;
>  	}
>  
>  	ret = snd_usb_lock_shutdown(subs->stream->chip);
>  	if (ret < 0)
> -		return ret;
> +		goto err_ret;
>  	ret = set_format(subs, fmt);
>  	snd_usb_unlock_shutdown(subs->stream->chip);
>  	if (ret < 0)
> -		return ret;
> +		goto err_ret;
>  
>  	subs->interface = fmt->iface;
>  	subs->altset_idx = fmt->altset_idx;
>  	subs->need_setup_ep = true;
>  
>  	return 0;
> +
> +err_ret:
> +	media_stop_pipeline(subs);
> +	return ret;
>  }
>  
>  /*
> @@ -759,6 +769,7 @@ static int snd_usb_hw_free(struct snd_pcm_substream *substream)
>  {
>  	struct snd_usb_substream *subs = substream->runtime->private_data;
>  
> +	media_stop_pipeline(subs);
>  	subs->cur_audiofmt = NULL;
>  	subs->cur_rate = 0;
>  	subs->period_bytes = 0;
> @@ -1219,6 +1230,7 @@ static int snd_usb_pcm_open(struct snd_pcm_substream *substream, int direction)
>  	struct snd_usb_stream *as = snd_pcm_substream_chip(substream);
>  	struct snd_pcm_runtime *runtime = substream->runtime;
>  	struct snd_usb_substream *subs = &as->substream[direction];
> +	int ret;
>  
>  	subs->interface = -1;
>  	subs->altset_idx = 0;
> @@ -1232,7 +1244,12 @@ static int snd_usb_pcm_open(struct snd_pcm_substream *substream, int direction)
>  	subs->dsd_dop.channel = 0;
>  	subs->dsd_dop.marker = 1;
>  
> -	return setup_hw_info(runtime, subs);
> +	ret = setup_hw_info(runtime, subs);
> +	if (ret == 0)
> +		ret = media_stream_init(subs, as->pcm, direction);
> +	if (ret)
> +		snd_usb_autosuspend(subs->stream->chip);
> +	return ret;
>  }
>  
>  static int snd_usb_pcm_close(struct snd_pcm_substream *substream, int direction)
> @@ -1241,6 +1258,7 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream, int direction)
>  	struct snd_usb_substream *subs = &as->substream[direction];
>  
>  	stop_endpoints(subs, true);
> +	media_stop_pipeline(subs);
>  
>  	if (subs->interface >= 0 &&
>  	    !snd_usb_lock_shutdown(subs->stream->chip)) {
> diff --git a/sound/usb/quirks-table.h b/sound/usb/quirks-table.h
> index 1a1e2e4..8f7b71b 100644
> --- a/sound/usb/quirks-table.h
> +++ b/sound/usb/quirks-table.h
> @@ -2875,6 +2875,7 @@ YAMAHA_DEVICE(0x7010, "UB99"),
>  		.product_name = pname, \
>  		.ifnum = QUIRK_ANY_INTERFACE, \
>  		.type = QUIRK_AUDIO_ALIGN_TRANSFER, \
> +		.media_device = 1, \
>  	} \
>  }
>  
> diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
> index 5ca80e7..b0d9d13 100644
> --- a/sound/usb/quirks.c
> +++ b/sound/usb/quirks.c
> @@ -36,6 +36,7 @@
>  #include "pcm.h"
>  #include "clock.h"
>  #include "stream.h"
> +#include "media.h"
>  
>  /*
>   * handle the quirks for the contained interfaces
> @@ -545,6 +546,10 @@ int snd_usb_create_quirk(struct snd_usb_audio *chip,
>  		[QUIRK_AUDIO_STANDARD_MIXER] = create_standard_mixer_quirk,
>  	};
>  
> +	if (quirk->media_device) {
> +		/* don't want to fail when media_device_create() fails */
> +		media_device_create(chip, iface);
> +	}
>  	if (quirk->type < QUIRK_TYPE_COUNT) {
>  		return quirk_funcs[quirk->type](chip, iface, driver, quirk);
>  	} else {
> diff --git a/sound/usb/stream.c b/sound/usb/stream.c
> index 8ee14f2..789e515 100644
> --- a/sound/usb/stream.c
> +++ b/sound/usb/stream.c
> @@ -36,6 +36,7 @@
>  #include "format.h"
>  #include "clock.h"
>  #include "stream.h"
> +#include "media.h"
>  
>  /*
>   * free a substream
> @@ -52,6 +53,7 @@ static void free_substream(struct snd_usb_substream *subs)
>  		kfree(fp);
>  	}
>  	kfree(subs->rate_list.list);
> +	media_stream_delete(subs);
>  }
>  
>  
> diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h
> index 15a1271..e3fac29 100644
> --- a/sound/usb/usbaudio.h
> +++ b/sound/usb/usbaudio.h
> @@ -109,6 +109,7 @@ struct snd_usb_audio_quirk {
>  	const char *product_name;
>  	int16_t ifnum;
>  	uint16_t type;
> +	bool media_device;
>  	const void *data;
>  };
>  
> -- 
> 2.5.0
> 
> 

^ permalink raw reply

* Re: [PATCH] dvb-usb-dvbsky: add new product id for TT CT2-4650 CI
From: Olli Salonen @ 2016-01-15 11:58 UTC (permalink / raw)
  To: Torbjorn Jansson; +Cc: linux-media
In-Reply-To: <5697DE47.4080902@mbox200.swipnet.se>

Hi Torbjörn,

No-one and everyone. :)

Feel free to make the changes you'd like to see. You probably have the
best picture of the changes between the devices.

Cheers,
-olli

On 14 January 2016 at 19:43, Torbjorn Jansson
<torbjorn.jansson@mbox200.swipnet.se> wrote:
> a question, who maintains the wiki?
> i think the wiki page for this card
> (http://www.linuxtv.org/wiki/index.php/TechnoTrend_TT-connect_CT2-4650_CI)
> should be updated.
>
> at the very least with some text saying that there is a second version of
> this card, possibly also something about it not yet working and i also think
> there is slightly different versions of tuner and demod in use and different
> firmware files is needed.
>
>
> On 2016-01-07 09:32, Olli Salonen wrote:
>>
>> Reviewed-by: Olli Salonen <olli.salonen@iki.fi>
>>
>> On 6 January 2016 at 17:26, Torbjörn Jansson
>> <torbjorn.jansson@mbox200.swipnet.se> wrote:
>>>
>>> Add a new product id to dvb-usb-dvbsky for new version of TechnoTrend
>>> CT2-4650 CI
>>>
>>> Signed-off-by: Torbjörn Jansson <torbjorn.jansson@mbox200.swipnet.se>
>>> ---
>>>   drivers/media/dvb-core/dvb-usb-ids.h  | 1 +
>>>   drivers/media/usb/dvb-usb-v2/dvbsky.c | 4 ++++
>>>   2 files changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/media/dvb-core/dvb-usb-ids.h
>>> b/drivers/media/dvb-core/dvb-usb-ids.h
>>> index 1c1c298..a3761e9 100644
>>> --- a/drivers/media/dvb-core/dvb-usb-ids.h
>>> +++ b/drivers/media/dvb-core/dvb-usb-ids.h
>>> @@ -247,6 +247,7 @@
>>>   #define USB_PID_TECHNOTREND_CONNECT_CT3650             0x300d
>>>   #define USB_PID_TECHNOTREND_CONNECT_S2_4600             0x3011
>>>   #define USB_PID_TECHNOTREND_CONNECT_CT2_4650_CI                0x3012
>>> +#define USB_PID_TECHNOTREND_CONNECT_CT2_4650_CI_2      0x3015
>>>   #define USB_PID_TECHNOTREND_TVSTICK_CT2_4400           0x3014
>>>   #define USB_PID_TERRATEC_CINERGY_DT_XS_DIVERSITY       0x005a
>>>   #define USB_PID_TERRATEC_CINERGY_DT_XS_DIVERSITY_2     0x0081
>>> diff --git a/drivers/media/usb/dvb-usb-v2/dvbsky.c
>>> b/drivers/media/usb/dvb-usb-v2/dvbsky.c
>>> index 1dd9625..b4620b7 100644
>>> --- a/drivers/media/usb/dvb-usb-v2/dvbsky.c
>>> +++ b/drivers/media/usb/dvb-usb-v2/dvbsky.c
>>> @@ -847,6 +847,10 @@ static const struct usb_device_id dvbsky_id_table[]
>>> = {
>>>                  USB_PID_TECHNOTREND_CONNECT_CT2_4650_CI,
>>>                  &dvbsky_t680c_props, "TechnoTrend TT-connect CT2-4650
>>> CI",
>>>                  RC_MAP_TT_1500) },
>>> +       { DVB_USB_DEVICE(USB_VID_TECHNOTREND,
>>> +               USB_PID_TECHNOTREND_CONNECT_CT2_4650_CI_2,
>>> +               &dvbsky_t680c_props, "TechnoTrend TT-connect CT2-4650 CI
>>> v1.1",
>>> +               RC_MAP_TT_1500) },
>>>          { DVB_USB_DEVICE(USB_VID_TERRATEC,
>>>                  USB_PID_TERRATEC_H7_3,
>>>                  &dvbsky_t680c_props, "Terratec H7 Rev.4",
>>> --
>>> 2.4.3
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-media" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>> --
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v3] media: v4l2-compat-ioctl32: fix missing length copy in put_v4l2_buffer32
From: Tiffany Lin @ 2016-01-15  9:13 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Hans Verkuil
  Cc: Eddie Huang, Yingjoe Chen, linux-media, linux-mediatek,
	Tiffany Lin

In v4l2-compliance utility, test QUERYBUF required correct length
value to go through each planar to check planar's length in
multi-planar buffer type

Signed-off-by: Tiffany Lin <tiffany.lin@mediatek.com>
---
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c |   21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
index 327e83a..6181470 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -426,10 +426,10 @@ static int get_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user
 					&up->timestamp.tv_usec))
 			return -EFAULT;
 
-	if (V4L2_TYPE_IS_MULTIPLANAR(kp->type)) {
-		if (get_user(kp->length, &up->length))
-			return -EFAULT;
+	if (get_user(kp->length, &up->length))
+		return -EFAULT;
 
+	if (V4L2_TYPE_IS_MULTIPLANAR(kp->type)) {
 		num_planes = kp->length;
 		if (num_planes == 0) {
 			kp->m.planes = NULL;
@@ -462,16 +462,14 @@ static int get_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user
 	} else {
 		switch (kp->memory) {
 		case V4L2_MEMORY_MMAP:
-			if (get_user(kp->length, &up->length) ||
-				get_user(kp->m.offset, &up->m.offset))
+			if (get_user(kp->m.offset, &up->m.offset))
 				return -EFAULT;
 			break;
 		case V4L2_MEMORY_USERPTR:
 			{
 			compat_long_t tmp;
 
-			if (get_user(kp->length, &up->length) ||
-			    get_user(tmp, &up->m.userptr))
+			if (get_user(tmp, &up->m.userptr))
 				return -EFAULT;
 
 			kp->m.userptr = (unsigned long)compat_ptr(tmp);
@@ -516,6 +514,9 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user
 		put_user(kp->reserved, &up->reserved))
 			return -EFAULT;
 
+	if (put_user(kp->length, &up->length))
+		return -EFAULT;
+
 	if (V4L2_TYPE_IS_MULTIPLANAR(kp->type)) {
 		num_planes = kp->length;
 		if (num_planes == 0)
@@ -536,13 +537,11 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user
 	} else {
 		switch (kp->memory) {
 		case V4L2_MEMORY_MMAP:
-			if (put_user(kp->length, &up->length) ||
-				put_user(kp->m.offset, &up->m.offset))
+			if (put_user(kp->m.offset, &up->m.offset))
 				return -EFAULT;
 			break;
 		case V4L2_MEMORY_USERPTR:
-			if (put_user(kp->length, &up->length) ||
-				put_user(kp->m.userptr, &up->m.userptr))
+			if (put_user(kp->m.userptr, &up->m.userptr))
 				return -EFAULT;
 			break;
 		case V4L2_MEMORY_OVERLAY:
-- 
1.7.9.5


^ permalink raw reply related

* Re: [RESEND PATCH v2] media: v4l2-compat-ioctl32: fix missing length copy in put_v4l2_buffer32
From: Hans Verkuil @ 2016-01-15  8:33 UTC (permalink / raw)
  To: Tiffany Lin, Mauro Carvalho Chehab
  Cc: Eddie Huang, Yingjoe Chen, linux-media, linux-mediatek
In-Reply-To: <1452828517-57392-1-git-send-email-tiffany.lin@mediatek.com>

On 01/15/2016 04:28 AM, Tiffany Lin wrote:
> In v4l2-compliance utility, test QUERYBUF required correct length
> value to go through each planar to check planar's length in
> multi-planar buffer type
> 
> Signed-off-by: Tiffany Lin <tiffany.lin@mediatek.com>
> ---
> Remove "Change-Id: I98faddc5711c24f17beda52e6d18c657add251ac"
> ---
> 
>  drivers/media/v4l2-core/v4l2-compat-ioctl32.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> index 327e83a..6c01920 100644
> --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> @@ -516,6 +516,9 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user
>  		put_user(kp->reserved, &up->reserved))
>  			return -EFAULT;
>  
> +	if (put_user(kp->length, &up->length))
> +		return -EFAULT;
> +

This should also be done for get_v4l2_buffer32, and the other places where
length is used in put/get_user() can now be removed.

Regards,

	Hans

>  	if (V4L2_TYPE_IS_MULTIPLANAR(kp->type)) {
>  		num_planes = kp->length;
>  		if (num_planes == 0)
> 


^ permalink raw reply

* [PATCH 2/3] [media] dvbdev: the space is required after ','
From: Xiubo Li @ 2016-01-15  5:14 UTC (permalink / raw)
  To: m.chehab; +Cc: linux-media, Xiubo Li
In-Reply-To: <1452834900-28360-1-git-send-email-lixiubo@cmss.chinamobile.com>

The space is missing after ',', and this will be introduce much
noise when checking new patch around them.

Signed-off-by: Xiubo Li <lixiubo@cmss.chinamobile.com>
---
 drivers/media/dvb-core/dvbdev.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index f38fabe..3b6e79e 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -58,7 +58,7 @@ static const char * const dnames[] = {
 #define DVB_MAX_IDS		MAX_DVB_MINORS
 #else
 #define DVB_MAX_IDS		4
-#define nums2minor(num,type,id)	((num << 6) | (id << 4) | type)
+#define nums2minor(num, type, id)	((num << 6) | (id << 4) | type)
 #define MAX_DVB_MINORS		(DVB_MAX_ADAPTERS*64)
 #endif
 
@@ -85,7 +85,7 @@ static int dvb_device_open(struct inode *inode, struct file *file)
 		file->private_data = dvbdev;
 		replace_fops(file, new_fops);
 		if (file->f_op->open)
-			err = file->f_op->open(inode,file);
+			err = file->f_op->open(inode, file);
 		up_read(&minor_rwsem);
 		mutex_unlock(&dvbdev_mutex);
 		return err;
@@ -867,7 +867,7 @@ int dvb_usercopy(struct file *file,
 			parg = sbuf;
 		} else {
 			/* too big to allocate from stack */
-			mbuf = kmalloc(_IOC_SIZE(cmd),GFP_KERNEL);
+			mbuf = kmalloc(_IOC_SIZE(cmd), GFP_KERNEL);
 			if (NULL == mbuf)
 				return -ENOMEM;
 			parg = mbuf;
-- 
1.8.3.1




^ permalink raw reply related

* [PATCH 1/3] [media] dvbdev: replace kcalloc with kzalloc
From: Xiubo Li @ 2016-01-15  5:14 UTC (permalink / raw)
  To: m.chehab; +Cc: linux-media, Xiubo Li
In-Reply-To: <1452834900-28360-1-git-send-email-lixiubo@cmss.chinamobile.com>

Since the number of elements equals to 1, so just use kzalloc to
simplify the code and make it more readable.

Signed-off-by: Xiubo Li <lixiubo@cmss.chinamobile.com>
---
 drivers/media/dvb-core/dvbdev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index 560450a..f38fabe 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -620,8 +620,7 @@ int dvb_create_media_graph(struct dvb_adapter *adap,
 			return -ENOMEM;
 		adap->conn = conn;
 
-		adap->conn_pads = kcalloc(1, sizeof(*adap->conn_pads),
-					    GFP_KERNEL);
+		adap->conn_pads = kzalloc(sizeof(*adap->conn_pads), GFP_KERNEL);
 		if (!adap->conn_pads)
 			return -ENOMEM;
 
-- 
1.8.3.1




^ permalink raw reply related

* [PATCH 0/3] [media] dvbdev: clean up the code
From: Xiubo Li @ 2016-01-15  5:14 UTC (permalink / raw)
  To: m.chehab; +Cc: linux-media, Xiubo Li


Xiubo Li (3):
  [media] dvbdev: replace kcalloc with kzalloc
  [media] dvbdev: the space is required after ','
  [media] dvbdev: remove useless parentheses after return

 drivers/media/dvb-core/dvbdev.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

-- 
1.8.3.1




^ permalink raw reply

* [PATCH 3/3] [media] dvbdev: remove useless parentheses after return
From: Xiubo Li @ 2016-01-15  5:15 UTC (permalink / raw)
  To: m.chehab; +Cc: linux-media, Xiubo Li
In-Reply-To: <1452834900-28360-1-git-send-email-lixiubo@cmss.chinamobile.com>

The parentheses are not required after return, and just remove it.

Signed-off-by: Xiubo Li <lixiubo@cmss.chinamobile.com>
---
 drivers/media/dvb-core/dvbdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index 3b6e79e..a6c26b5 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -352,7 +352,7 @@ static int dvb_create_media_entity(struct dvb_device *dvbdev,
 	ret = media_device_register_entity(dvbdev->adapter->mdev,
 					   dvbdev->entity);
 	if (ret)
-		return (ret);
+		return ret;
 
 	printk(KERN_DEBUG "%s: media entity '%s' registered.\n",
 		__func__, dvbdev->entity->name);
-- 
1.8.3.1




^ permalink raw reply related

* cron job: media_tree daily build: OK
From: Hans Verkuil @ 2016-01-15  4:01 UTC (permalink / raw)
  To: linux-media

This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:		Fri Jan 15 04:00:25 CET 2016
git branch:	test
git hash:	768acf46e1320d6c41ed1b7c4952bab41c1cde79
gcc version:	i686-linux-gcc (GCC) 5.1.0
sparse version:	v0.5.0-51-ga53cea2
smatch version:	v0.5.0-3228-g5cf65ab
host hardware:	x86_64
host os:	4.3.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-exynos: OK
linux-git-arm-mx: OK
linux-git-arm-omap: OK
linux-git-arm-omap1: OK
linux-git-arm-pxa: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.34.7-i686: OK
linux-2.6.35.9-i686: OK
linux-2.6.36.4-i686: OK
linux-2.6.37.6-i686: OK
linux-2.6.38.8-i686: OK
linux-2.6.39.4-i686: OK
linux-3.0.60-i686: OK
linux-3.1.10-i686: OK
linux-3.2.37-i686: OK
linux-3.3.8-i686: OK
linux-3.4.27-i686: OK
linux-3.5.7-i686: OK
linux-3.6.11-i686: OK
linux-3.7.4-i686: OK
linux-3.8-i686: OK
linux-3.9.2-i686: OK
linux-3.10.1-i686: OK
linux-3.11.1-i686: OK
linux-3.12.23-i686: OK
linux-3.13.11-i686: OK
linux-3.14.9-i686: OK
linux-3.15.2-i686: OK
linux-3.16.7-i686: OK
linux-3.17.8-i686: OK
linux-3.18.7-i686: OK
linux-3.19-i686: OK
linux-4.0-i686: OK
linux-4.1.1-i686: OK
linux-4.2-i686: OK
linux-4.3-i686: OK
linux-4.4-i686: OK
linux-2.6.34.7-x86_64: OK
linux-2.6.35.9-x86_64: OK
linux-2.6.36.4-x86_64: OK
linux-2.6.37.6-x86_64: OK
linux-2.6.38.8-x86_64: OK
linux-2.6.39.4-x86_64: OK
linux-3.0.60-x86_64: OK
linux-3.1.10-x86_64: OK
linux-3.2.37-x86_64: OK
linux-3.3.8-x86_64: OK
linux-3.4.27-x86_64: OK
linux-3.5.7-x86_64: OK
linux-3.6.11-x86_64: OK
linux-3.7.4-x86_64: OK
linux-3.8-x86_64: OK
linux-3.9.2-x86_64: OK
linux-3.10.1-x86_64: OK
linux-3.11.1-x86_64: OK
linux-3.12.23-x86_64: OK
linux-3.13.11-x86_64: OK
linux-3.14.9-x86_64: OK
linux-3.15.2-x86_64: OK
linux-3.16.7-x86_64: OK
linux-3.17.8-x86_64: OK
linux-3.18.7-x86_64: OK
linux-3.19-x86_64: OK
linux-4.0-x86_64: OK
linux-4.1.1-x86_64: OK
linux-4.2-x86_64: OK
linux-4.3-x86_64: OK
linux-4.4-x86_64: OK
apps: OK
spec-git: OK
sparse: WARNINGS
smatch: ERRORS

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Friday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Friday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/media.html

^ permalink raw reply

* [RESEND PATCH v2] media: v4l2-compat-ioctl32: fix missing length copy in put_v4l2_buffer32
From: Tiffany Lin @ 2016-01-15  3:28 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Hans Verkuil
  Cc: Eddie Huang, Yingjoe Chen, linux-media, linux-mediatek,
	Tiffany Lin

In v4l2-compliance utility, test QUERYBUF required correct length
value to go through each planar to check planar's length in
multi-planar buffer type

Signed-off-by: Tiffany Lin <tiffany.lin@mediatek.com>
---
Remove "Change-Id: I98faddc5711c24f17beda52e6d18c657add251ac"
---

 drivers/media/v4l2-core/v4l2-compat-ioctl32.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
index 327e83a..6c01920 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -516,6 +516,9 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user
 		put_user(kp->reserved, &up->reserved))
 			return -EFAULT;
 
+	if (put_user(kp->length, &up->length))
+		return -EFAULT;
+
 	if (V4L2_TYPE_IS_MULTIPLANAR(kp->type)) {
 		num_planes = kp->length;
 		if (num_planes == 0)
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH] media: v4l2-compat-ioctl32: fix missing length copy in put_v4l2_buffer32
From: tiffany lin @ 2016-01-15  3:05 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Mauro Carvalho Chehab, Eddie Huang, Yingjoe Chen, linux-media,
	linux-mediatek
In-Reply-To: <56975409.3090801@xs4all.nl>

Hi Hans,

On Thu, 2016-01-14 at 08:53 +0100, Hans Verkuil wrote:
> Hi Tiffany,
> 
> Good catch! But looking through the compat code I noticed that in the
> single-planar DMABUF case the length field is also never copied. I think
> it would be much simpler if the length field is just always copied instead
> of in each multiplanar or singleplanar mmap/userptr/dmabuf case. It will
> simplify the code and prevent such mistakes in the future.
> 
> Can you make a v2 that makes that change?
> 
Sure, I had sent v2.

best regards,
Tiffany
> Thanks!
> 
> 	Hans
> 
> On 01/14/2016 08:37 AM, Tiffany Lin wrote:
> > In v4l2-compliance utility, test QUERYBUF required correct length
> > value to go through each planar to check planar's length in multi-planar
> > 
> > Signed-off-by: Tiffany Lin <tiffany.lin@mediatek.com>
> > ---
> >  drivers/media/v4l2-core/v4l2-compat-ioctl32.c |    3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > index 327e83a..5ba932a 100644
> > --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > @@ -521,6 +521,9 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user
> >  		if (num_planes == 0)
> >  			return 0;
> >  
> > +		if (put_user(kp->length, &up->length))
> > +			return -EFAULT;
> > +
> >  		uplane = (__force struct v4l2_plane __user *)kp->m.planes;
> >  		if (get_user(p, &up->m.planes))
> >  			return -EFAULT;
> > 
> 



^ permalink raw reply

* [PATCH v2] media: v4l2-compat-ioctl32: fix missing length copy in put_v4l2_buffer32
From: Tiffany Lin @ 2016-01-15  2:58 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Hans Verkuil
  Cc: Eddie Huang, Yingjoe Chen, linux-media, linux-mediatek,
	Tiffany Lin

In v4l2-compliance utility, test QUERYBUF required correct length
value to go through each planar to check planar's length in
multi-planar buffer type

Change-Id: I98faddc5711c24f17beda52e6d18c657add251ac
Signed-off-by: Tiffany Lin <tiffany.lin@mediatek.com>
---
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
index 327e83a..6c01920 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -516,6 +516,9 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user
 		put_user(kp->reserved, &up->reserved))
 			return -EFAULT;
 
+	if (put_user(kp->length, &up->length))
+		return -EFAULT;
+
 	if (V4L2_TYPE_IS_MULTIPLANAR(kp->type)) {
 		num_planes = kp->length;
 		if (num_planes == 0)
-- 
1.7.9.5


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox