Re: [RFC PATCH] uvcvideo: Add mapping for HEVC payloads

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Dmitry,

Sorry for the late reply.

Mauro, there's a question for your below.

On Tue, Jul 28, 2020 at 09:25:46AM +1000, Dmitry Buzdyk wrote:
> Hi Laurent,
> 
> Had you a chance to review USB descriptors from the device provided
> below? Is any additional information needed?
> 
> On Wed, Jul 15, 2020 at 06:00:10PM +1000, Dmitry Buzdyk wrote:
> > On Tue, Jun 09, 2020 at 02:57:36PM +1000, Dmitry Buzdyk wrote:
> > Hi Laurent,
> > 
> > Please see updated information below
> > 
> >> On Sun, Jun 07, 2020 at 04:07:19AM +0300, Laurent Pinchart wrote:
> >>> Hi Dmitry,
> >>> 
> >>> Thank you for the patch.
> >>> 
> >>> On Fri, May 29, 2020 at 11:05:47AM +1000, Dmitry Buzdyk wrote:
> >>>> Add HEVC GUID and assotiate with HEVC pixel format so that frame
> >>>> based format descriptors recognized by the UVC video driver.
> >>> 
> >>> The patch itself looks OK to me, but could you share a bit more
> >>> information about which device(s) implement this ? Are they UVC 1.1
> >>> devices ? Could you share their full USB descriptors (retrieved with
> >>> 'lsusb -v', running as root if possible) ?
> >>
> >> This is a UVC1.1 camera device based on Ambarella H22 SOC.

That's interesting. It would be nice to have upstream support for the
Ambarella SoCs in the kernel :-)

> >> Please note that device is still in development and yet to get actual
> >> VID and PID.
> >
> > Device got its VID:PID from USB-IF:
> > 
> > Bus 001 Device 009: ID 3301:1000  
> > Device Descriptor:
> >   bLength                18
> >   bDescriptorType         1
> >   bcdUSB               2.00
> >   bDeviceClass          239 Miscellaneous Device
> >   bDeviceSubClass         2 
> >   bDeviceProtocol         1 Interface Association
> >   bMaxPacketSize0        64
> >   idVendor           0x3301 
> >   idProduct          0x1000 
> >   bcdDevice            0.10
> >   iManufacturer           1 Rhonda
> >   iProduct                2 Rhonda Cam
> >   iSerial                 3 FMABCLE15000007
> >   bNumConfigurations      1

Thank you for the descriptors.

[snip]

> >       VideoControl Interface Descriptor:
> >         bLength                 9
> >         bDescriptorType        36
> >         bDescriptorSubtype      3 (OUTPUT_TERMINAL)
> >         bTerminalID            16
> >         wTerminalType      0x0101 USB Streaming
> >         bAssocTerminal          0
> >         bSourceID              10
> >         iTerminal               0 
> >       VideoControl Interface Descriptor:
> >         bLength                 9
> >         bDescriptorType        36
> >         bDescriptorSubtype      3 (OUTPUT_TERMINAL)
> >         bTerminalID            17
> >         wTerminalType      0x0101 USB Streaming
> >         bAssocTerminal          0
> >         bSourceID              10
> >         iTerminal               0 

Two output terminals ? Does that mean the device can provide two streams
from the same source ?

[snip]


> >       Endpoint Descriptor:
> >         bLength                 7
> >         bDescriptorType         5
> >         bEndpointAddress     0x83  EP 3 IN
> >         bmAttributes            2
> >           Transfer Type            Bulk
> >           Synch Type               None
> >           Usage Type               Data
> >         wMaxPacketSize     0x0200  1x 512 bytes
> >         bInterval               0

This is interesting too, does it provide enough bandwidth for 3000x4000
@10fps MJPEG ?

[snip]

> >>> Is there anything else needed to get HEVC capture working, such as
> >>> extension unit controls, or is this patch enough ? What userspace
> >>> software do you use to capture and decode HEVC (or capture it to disk) ?
> >>
> >> No other changes to Linux nor extra actions needed to start HEVC capture.
> >> We use patched version of FFmpeg to capture, decode and display HEVC
> >> stream from camera device. That simple patch also going to be sent to
> >> FFmpeg upstream.
> >
> > Patch for FFmpeg sent to https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=1760
> >
> >>>> Signed-off-by: Dmitry Buzdyk <dima.buzdyk@gmail.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

and taken in my tree. I'm afraid we're a bit too close to the v5.9 merge
window for me to send a pull request now, unless Mauro would be fine
with that. Otherwise I'll include it in the pull request for the next
release.

> >>>> ---
> >>>>  drivers/media/usb/uvc/uvc_driver.c | 5 +++++
> >>>>  drivers/media/usb/uvc/uvcvideo.h   | 4 ++++
> >>>>  2 files changed, 9 insertions(+)
> >>>> 
> >>>> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> >>>> index 431d86e1c94b..825ee3601661 100644
> >>>> --- a/drivers/media/usb/uvc/uvc_driver.c
> >>>> +++ b/drivers/media/usb/uvc/uvc_driver.c
> >>>> @@ -214,6 +214,11 @@ static struct uvc_format_desc uvc_fmts[] = {
> >>>>  		.guid		= UVC_GUID_FORMAT_CNF4,
> >>>>  		.fcc		= V4L2_PIX_FMT_CNF4,
> >>>>  	},
> >>>> +	{
> >>>> +		.name		= "HEVC",
> >>>> +		.guid		= UVC_GUID_FORMAT_HEVC,
> >>>> +		.fcc		= V4L2_PIX_FMT_HEVC,
> >>>> +	},
> >>>>  };
> >>>>  
> >>>>  /* ------------------------------------------------------------------------
> >>>> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> >>>> index 6ab972c643e3..c7f043121b41 100644
> >>>> --- a/drivers/media/usb/uvc/uvcvideo.h
> >>>> +++ b/drivers/media/usb/uvc/uvcvideo.h
> >>>> @@ -165,6 +165,10 @@
> >>>>  	{0x32, 0x00, 0x00, 0x00, 0x02, 0x00, 0x10, 0x00, \
> >>>>  	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> >>>>  
> >>>> +#define UVC_GUID_FORMAT_HEVC \
> >>>> +	{ 'H',  'E',  'V',  'C', 0x00, 0x00, 0x10, 0x00, \
> >>>> +	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> >>>> +
> >>>>  
> >>>>  /* ------------------------------------------------------------------------
> >>>>   * Driver specific constants.

-- 
Regards,

Laurent Pinchart