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
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
