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? -- John GnuPG fingerprint: D0EC B3DB C372 D1F1 0B01 83F0 49F1 D7B2 60D4 D0F7
signature.asc
Description: OpenPGP digital signature
_______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
