[FFmpeg-devel] [PATCH][WIP][RFC] Add support for MPEG-4 Simple Studio Profile

Michael Niedermayer michael at niedermayer.cc
Sun Jan 8 01:42:14 EET 2017


On Sat, Jan 07, 2017 at 10:35:43PM +0000, Kieran Kunhya wrote:
> Hi,
> 
> I have added support for MPEG-4 Sstp using the available samples on trac.
> Yes it doesn't pass fate, yes it's not format-patch, yes it uses printfs.
> https://trac.ffmpeg.org/ticket/4447
> 
> Being MPEG-4, it depends on mpegvideo.c so has tons of yuv420p assumptions
> baked in which are of course undocumented.
> Here are my questions (line number refers to attached patch):
> 
> Line 35: How do I signal to this idctdsp thing that I want an idct with
> 32-bit coefficients AND intermediates? A lot of that code has assumptions
> that intermediates will be the next size up, i.e 8-bit coeffs, 16-bit
> intermediates, or 16-bit coeffs and 32-bit intermediates.
> 

> Line 906: Why do RGB samples not decode unless the GBRP format is moved to
> the top of PIX_FMT. I get "[mpeg4 @ 0x7f945c029600] format change not
> supported" otherwise.

The format is choosen by the AVCodecContext.get_format() callback
from the list of choices a decoder presents to it.
if you present yuv420 as a choice it can pick that.
the current code will present the full list from the AVCodec as is
and the first in the list is supposed to be the best choice so it
likely will be choosen


> 
> Line 932: What's going on with this branch. Normal mpeg-4 video does
> dequant during unpack, why is it not part of this condition?

mpeg-4 uses DC/AC prediction in intra blocks, this prediction is done
before dequantization so doing dequant during block decode becomes
messy, its no longer just the non zero coded coeffs that need dequant

and as the dequant codepath is needed for encode already, using it for
intra blocks too is the cleanest solution. Doing decode+pred+dequant
in one would be probably rather ugly


> 
> Line 987: Are there more assumptions baked into mpegvideo.c about "square"
> macroblocks, i.e ones where (width == height)?

a code "audit" would tell if there are more such assumptions, i dont
know of the top of my head, its too long ago


> 
> Line 997: What is all this stuff going on with -1U, unless I remove this I
> get a segfault. I do get a stripe on the left though.

IIRC the 1U compensates the effect of ff_update_block_index()
block_size in ff_update_block_index() looks wrong after your patch

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170108/85b4b6cf/attachment.sig>


More information about the ffmpeg-devel mailing list