raspberrypi / firmware

This repository contains pre-compiled binaries of the current Raspberry Pi kernel and modules, userspace libraries, and bootloader/GPU firmware.
5.18k stars 1.68k forks source link

mmal/image_fx: deinterlace not deinterlacing picture #889

Open julianscheel opened 7 years ago

julianscheel commented 7 years ago

I'm seeing a problem where the deinterlace filter is not actually deinterlacing a picture if the codec detected it as progressive. This video is actually interlaced, but the codec detects it as progressive. To enforce deinterlacing I tried to set the MMAL_BUFFER_HEADER_VIDEO_FLAG_INTERLACED and MMAL_BUFFER_HEADER_VIDEO_FLAG_TOP_FIELD_FIRST flags, so the buffer->flags field is 0x30004 before sending to the image_fx input port, but it still does not deinterlace the pictures. Is image_fx using any other sources than these flags to decide whether deinterlacing shall be applied or not? Pictures are passed between codec and image_fx as opaque in case this matters.

julianscheel commented 7 years ago

I see this commit https://github.com/raspberrypi/firmware/commit/b2c05bd1c7194f47193b52bc366fec9ed0048af5 adds "deinterlace: Provide a mode where frame flags are exclusively used". Sounds like this could be what I need, but I couldn't spot any details on how to enable that mode...

julianscheel commented 7 years ago

@popcornmix Any thoughts on this? Sorry for being impatient :)

julianscheel commented 6 years ago

ping @popcornmix

julianscheel commented 6 years ago

@popcornmix Any chance you could take a look at this? It is a hard blocker for us, preventing us from upgrading the firmware.

JamesH65 commented 6 years ago

@6by9 Is a better bet for this.

6by9 commented 6 years ago

Actually @popcornmix is the expert on deinterlacing, not me.

Having a quick look at the code, https://github.com/raspberrypi/firmware/commit/b2c05bd1c7194f47193b52bc366fec9ed0048af5 is adding an extra value that can be passed via OMX_IndexConfigCommonImageFilterParameters / MMAL_PARAMETER_IMAGE_EFFECT_PARAMETERS. It takes up to 4 parameters:

Expected integral values for the first parameter:
   0 - The data is not interlaced, it is progressive scan
   1 - The data is interlaced, fields sent separately in temporal order, with upper field first
   2 - The data is interlaced, fields sent separately in temporal order, with lower field first
   3 - The data is interlaced, two fields sent together line interleaved, with the upper field temporally earlier
   4 - The data is interlaced, two fields sent together line interleaved, with the lower field temporally earlier
   5 - The stream may contain a mixture of progressive and interlaced frames (all bets are off).

Parameter 2 is default_frame_interval Parameter 3 is half_frame_rate Parameter 4 is use_qpus A value of 0 in any field is treated as "use the default/compute for yourself".

That commit was adding mode 5 to parameter 1, and appears to be the mode used by Kodi: https://github.com/xbmc/xbmc/blob/master/xbmc/cores/VideoPlayer/VideoRenderers/HwDecRender/MMALRenderer.cpp#L1373 Exact details on behaviour would take a bit longer to deduce.

JamesH65 commented 5 years ago

@julianscheel I know its been a while, did you ever sort this out? Can the issue be closed or is there more to be done?

julianscheel commented 5 years ago

@JamesH65 I don't think it was ever resolved. We did simply not update out firmware any further...