On Sat, 7 Jan 2017 at 23:43 Michael Niedermayer <mich...@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@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
