On Sun, Jan 08, 2017 at 01:00:31AM +0000, Kieran Kunhya wrote: > 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?
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
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
