On Mon, 25 Nov 2013 09:14:17 -0800, John Stebbins <[email protected]> wrote: > On 11/25/2013 09:02 AM, Anton Khirnov wrote: > > On Mon, 25 Nov 2013 08:15:28 -0800, John Stebbins <[email protected]> > > wrote: > >> On 11/24/2013 11:08 PM, Anton Khirnov 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. > >> Would you prefer that this be the byte count? Taking a second look at it, > >> this may be more obvious and doesn't really > >> have much effect on the CC decoder side of things. > > I don't know what I'd prefer because I still don't know what does count > > mean ;) > > Is it a number of "elements" (whatever those are)? > > Yes, it is currently the number of elements which are a fixed size of 3 bytes > per element. > > > > > Does the caller even need to know it? Perhaps it'd be better to make this > > just a > > plain uint8_t* array instead of a struct. > > The caller only needs to know it for the purposes of passing it along to a CC > decoder. So yes, we could certainly just > use AVFrameSideData.size for this. I would have to add documentation around > the new side data type AV_FRAME_DATA_A53_CC > to make it clear that AVFrameSideData.size specifies the number of CC bytes, > but it eliminates duplicate information > that isn't strictly necessary. Is this what you would prefer? >
Yes, that sounds better to me. -- Anton Khirnov _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
