[FFmpeg-devel] [PATCH 09/12] avformat/mxfdec: add support for clip wrapped essences

[Marton Balint]

On Thu, 14 Jun 2018, Marton Balint wrote:

>
>
> On Thu, 14 Jun 2018, Tomas Härdin wrote:
>
>> ons 2018-06-13 klockan 22:11 +0200 skrev Marton Balint:
>>> 
>>> On Wed, 13 Jun 2018, Tomas Härdin wrote:
>>> 
>>> > sön 2018-06-10 klockan 12:36 +0200 skrev Marton Balint:
>>> > > Also use common code with opAtom.
>>> > > 
>>> > > Fixes ticket #2776.
>>> > > Partially fixes ticket #5671.
>>> > > Fixes ticket #5866.
>>> > > 
>>> > > > Signed-off-by: Marton Balint <cus at passwd.hu>
>>> > > 
>>> > > ---
>>> > >  libavformat/mxfdec.c | 281 
> ++++++++++++++++++++++++---------------------------
>>> > >  1 file changed, 130 insertions(+), 151 deletions(-)
>>> > > 
>>> > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
>>> > >  
>>> > > -            next_ofs = mxf_set_current_edit_unit(mxf, klv.offset);
>>> > > -
>>> > > -            if (next_ofs >= 0 && klv.next_klv > next_ofs) {
>>> > > -                /* if this check is hit then it's possible OPAtom was 
> treated as OP1a
>>> > > -                 * truncate the packet since it's probably very large 
> (>2 GiB is common) */
>>> > > -                avpriv_request_sample(s,
>>> > > -                                      "OPAtom misinterpreted as OP1a? 
> "
>>> > > -                                      "KLV for edit unit %"PRId64" 
> extending into "
>>> > > -                                      "next edit unit",
>>> > > -                                      mxf->current_edit_unit);
>>> > > -                klv.length = next_ofs - avio_tell(s->pb);
>>> > > +            next_ofs = mxf_set_current_edit_unit(mxf, st, pos, 1);
>>> > > +
>>> > > +            if (track->wrapping != FrameWrapped) {
>>> > > +                int64_t size;
>>> > > +
>>> > > +                if (next_ofs <= 0) {
>>> > > +                    // If we have no way to packetize the data, then 
> return it in chunks...
>>> > > +                    int64_t max_packet_size = 33554432;
>>> > 
>>> > Any reason for this particular number?
>>> 
>>> Not really, I chose 32 MB because it is big enough to not fragment 
> typical 
>>> exotic files (e.g.: GOP wrapped, etc), but small enough so allocating 
>>> this much RAM in one packet does not cause any issues if we encounter a 
>>> huge KLV with some unknown (clip?) wrapping.
>>
>> Perhaps add a #define MXF_MAX_CHUNK_SIZE (32 << 20) or so on the top of
>> the file
>
> Sure.
>
>>
>>> To be frank, I am still not sure if this should actually work or not, or 
>>> if the parser system of libavformat can make something useful out of a 
>>> chunked source like this or not, I don't seem to have an MXF file which 
>>> triggers this.
>>
>> There's no way to split it into edit units based on duration? Surely
>> outputing packets of size klv.length / duration should be fine? If
>> duration == 1 then surely the entire chunk should be read? It's easy
>> enough imagine DPX being wrapped inside MXF, and DPX frames can easily
>> be 50+ MiB. Take 8k raw 8-bit 4:4:4 video for example. 8192 x 4608 x 3
>> = 108 MiB. An error could be output if klv.size % duration != 0.
>
> That is a good idea in general, but I think it is better if we implement 
> it in mxf_handle_missing_index_table_segment and generate a fake index 
> table segment with a constant edit unit byte count for clip wrapped 
> essences without a proper index.

Ok, pushed patches 1-8, 12, and the additional "fix" patches I sent.

I will resend the remaining with index guessing support.

Thanks everybody for comments and testing.

Regards,
Marton