[FFmpeg-devel] [PATCH 1/2] Parse MXF partitions
Tomas Härdin
tomas.hardin at codemill.se
Tue May 17 16:44:08 CEST 2011
Forked from the thread "[PATCH] MXF index table based seeking" to avoid
cluttering the original thread up.
Baptiste Coudurier skrev 2011-05-17 05:31:
>> typedef struct {
>> + MXFPartition **partitions;
>> + unsigned partitions_count;
>
> Any reason you don't use MXFPartition *partitions ?
> It avoids malloc for each partition.
No reason at all. Reworked.
>> [...]
>>
>> +
>> + /* consider both footers to be closed (there is only Footer and
CompleteFooter) */
>> + partition->closed = partition->type == Footer || !(uid[14]& 1);
>
> Do we really need to workaround here ?
Not really a workaround, just a statement of fact. There's no such thing
as OpenFooter, hence we might as well set closed correctly.
>> [...]
>>
>> +
>> + /* only grab OP from the header */
>> + if (partition->type != Header)
>> + return 0;
>
> Why only header ? :)
Why not? :) I don't feel strongly about it though.
I removed the check and reworked the part below it since I discovered it
was lying - it claims the demuxer supports OPAtom. This is something we
can hopefully rectify soon though.
The attached patch also parses the remaining operational patterns out,
so we can detect OP1a up to OP3c and OPAtom.
>> [...]
>>
>> @@ -843,6 +931,16 @@ static int
mxf_parse_structural_metadata(MXFContext *mxf)
>>
>> static const MXFMetadataReadTableEntry mxf_metadata_read_table[] = {
>> { {
0x06,0x0E,0x2B,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x05,0x01,0x00
}, mxf_read_primer_pack },
>> + { {
0x06,0x0E,0x2B,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x02,0x01,0x00
}, mxf_read_partition_pack },
>> + { {
0x06,0x0E,0x2B,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x02,0x02,0x00
}, mxf_read_partition_pack },
>> + { {
0x06,0x0E,0x2B,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x02,0x03,0x00
}, mxf_read_partition_pack },
>> + { {
0x06,0x0E,0x2B,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x02,0x04,0x00
}, mxf_read_partition_pack },
>> + { {
0x06,0x0E,0x2B,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x03,0x01,0x00
}, mxf_read_partition_pack },
>> + { {
0x06,0x0E,0x2B,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x03,0x02,0x00
}, mxf_read_partition_pack },
>> + { {
0x06,0x0E,0x2B,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x03,0x03,0x00
}, mxf_read_partition_pack },
>> + { {
0x06,0x0E,0x2B,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x03,0x04,0x00
}, mxf_read_partition_pack },
>> + { {
0x06,0x0E,0x2B,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x04,0x02,0x00
}, mxf_read_partition_pack },
>> + { {
0x06,0x0E,0x2B,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x04,0x04,0x00
}, mxf_read_partition_pack },
>
> Do we need to parse non complete partitions ?
> Does it happen in the wild ? :/
Open headers and such aren't unheard of. It doesn't hurt parsing them I
think.
Preferrably we'd start by parsing any metadata we come across (even open
partitions), but override the metadata parsed when we come across a
complete header. I think that discussion belongs to [PATCH 2/2] though.
> Anyway, patch ok, nice work :)
>
> I'll review the other patch later.
Updated patch attached
/Tomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Parse-MXF-partitions.patch
Type: text/x-patch
Size: 6643 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110517/5c5e3f92/attachment.bin>
More information about the ffmpeg-devel
mailing list