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

[Kieran Kunhya]

On Sat, 7 Jan 2017 at 23:43 Michael Niedermayer <michael at niedermayer.cc>
wrote:

> 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
>

Why does it ignore the pix_fmt I tell it to use?

>
> > 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
>

Well I segfault with the -1U so what do I do? How can this happen?

Kieran


>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Republics decline into democracies and democracies degenerate into
> despotisms. -- Aristotle
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>