On Fri, 2015-07-17 at 12:36 +0200, Carl Eugen Hoyos wrote:
> On Saturday 11 July 2015 04:13:52 pm Tomas Härdin wrote:
> > Just a quick review since I have to bounce:
> > > +const MXFCodecUL ff_mxf_codec_tag_uls[] = {
> >
> > Haven't we moved this to mxf.c already? Or rather, don't we 
> > have a whole bunch of very similar tables already?
> 
> The new table is (together with others) in mxf.c, none of the 
> existing tables maps to codec_tag. AVup cannot be decoded 
> without codec_tag because we currently treat it as rawvideo.

Alright

> [...]
> 
> > Messy bracing. Something like putting a check on codec_tag after setting
> > it, inside braces like before. Hard to explain and I don't have time to
> > type it out but:
> >
> > if (st->codec->pix_fmt == AV_PIX_FMT_NONE) {
> >  codec_tag = ...
> >  if (!codec_tag) {
> >   do the old thing
> >  }
> > }
> 
> New patch attached.

Looks alright. You can reindent in a separate patch if you like.

Sorry for taking a few days with replying, thought I'd already sorted it

/Tomas

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to