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

[Michael Niedermayer]

On Sun, Jan 08, 2017 at 01:00:31AM +0000, Kieran Kunhya wrote:
> 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?

its likely set by h263_get_format() calling ff_get_format()
calling avctx->get_format() / avcodec_default_get_format()

I would try to pass the correct list of pixel formats to
ff_get_format(), that is only 422 formats if its 422, only 444 formats
if its 444, only rgb formats if its rgb [it could be multiple because
of hw decoders]


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

ff_update_block_index() must update dest by the correct size of the
macroblock
that is unless iam missing something, which is not impossible


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

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
-------------- 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/fe7f46ef/attachment.sig>