On Mon, Nov 25, 2013 at 8:08 AM, Anton Khirnov <[email protected]> wrote: > > On Sun, 24 Nov 2013 09:49:47 -0800, John Stebbins <[email protected]> > wrote: >> --- >> libavcodec/avcodec.h | 5 ++++ >> libavcodec/mpeg12dec.c | 64 >> ++++++++++++++++++++++++++++++++++++++++++++++++++ >> libavutil/frame.h | 4 ++++ >> 3 files changed, 73 insertions(+) >> >> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h >> index 4ce6d61..9e7d968 100644 >> --- a/libavcodec/avcodec.h >> +++ b/libavcodec/avcodec.h >> @@ -845,6 +845,11 @@ typedef struct AVPanScan{ >> int16_t position[3][2]; >> }AVPanScan; >> >> +typedef struct AVClosedCaption { >> + int count; >> + uint8_t data[1]; >> +} AVClosedCaption; > > First, those two fields should be documented. Perhaps it's obvious to you what > they mean, but it's not so obvious to me. E.g. my first assumption would be > that > count is the size of data in bytes, but looking at the code it is not so. > > Second, this is apparrently a dump of some specific wire format, right? Then > it > should be documented which one it is and the struct and the side data type > should have more specific names, so we can add other CC formats later. >
Also, is it right to put this in avcodec? Maybe there can be filters and/or utilities that might want to play with closed captions, but I'm not familiar enough with the subject to say whether that's correct or not. I recommend a MICRO bump of the library this gets added to anyway. Cheers, Vittorio _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
