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

Submitted by Marton Balint on June 10, 2018, 10:36 a.m.

Details

Message ID 20180610103650.10155-9-cus@passwd.hu
State New
Headers show

Commit Message

Marton Balint June 10, 2018, 10:36 a.m.
Also use common code with opAtom.

Fixes ticket #2776.
Partially fixes ticket #5671.
Fixes ticket #5866.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/mxfdec.c | 281 ++++++++++++++++++++++++---------------------------
 1 file changed, 130 insertions(+), 151 deletions(-)

Comments

Michael Niedermayer June 11, 2018, 11:09 p.m.
On Sun, Jun 10, 2018 at 12:36:47PM +0200, Marton Balint wrote:
> Also use common code with opAtom.
> 
> Fixes ticket #2776.
> Partially fixes ticket #5671.
> Fixes ticket #5866.
> 
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavformat/mxfdec.c | 281 ++++++++++++++++++++++++---------------------------
>  1 file changed, 130 insertions(+), 151 deletions(-)

causes a segfault:

==23735== Invalid read of size 8
==23735==    at 0x75A627: mxf_set_pts (mxfdec.c:3277)
==23735==    by 0x75ACAD: mxf_read_packet_old (mxfdec.c:3396)
==23735==    by 0x7E099D: ff_read_packet (utils.c:856)
==23735==    by 0x7E39FF: read_frame_internal (utils.c:1581)
==23735==    by 0x7EB82B: avformat_find_stream_info (utils.c:3773)
==23735==    by 0x415534: open_input_file (ffmpeg_opt.c:1091)
==23735==    by 0x41EB11: open_files (ffmpeg_opt.c:3206)
==23735==    by 0x41ECA3: ffmpeg_parse_options (ffmpeg_opt.c:3246)
==23735==    by 0x43D1A3: main (ffmpeg.c:4832)
==23735==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==23735== 
==23735== 
==23735== Process terminating with default action of signal 11 (SIGSEGV)
==23735==  Access not within mapped region at address 0x0
==23735==    at 0x75A627: mxf_set_pts (mxfdec.c:3277)
==23735==    by 0x75ACAD: mxf_read_packet_old (mxfdec.c:3396)
==23735==    by 0x7E099D: ff_read_packet (utils.c:856)
==23735==    by 0x7E39FF: read_frame_internal (utils.c:1581)
==23735==    by 0x7EB82B: avformat_find_stream_info (utils.c:3773)
==23735==    by 0x415534: open_input_file (ffmpeg_opt.c:1091)
==23735==    by 0x41EB11: open_files (ffmpeg_opt.c:3206)
==23735==    by 0x41ECA3: ffmpeg_parse_options (ffmpeg_opt.c:3246)
==23735==    by 0x43D1A3: main (ffmpeg.c:4832)
==23735==  If you believe this happened as a result of a stack
==23735==  overflow in your program's main thread (unlikely but
==23735==  possible), you can try to increase the size of the
==23735==  main thread stack using the --main-stacksize= flag.
==23735==  The main thread stack size used in this run was 8388608.

[...]
Marton Balint June 12, 2018, 8:47 a.m.
On Tue, 12 Jun 2018, Michael Niedermayer wrote:

> On Sun, Jun 10, 2018 at 12:36:47PM +0200, Marton Balint wrote:
>> Also use common code with opAtom.
>>
>> Fixes ticket #2776.
>> Partially fixes ticket #5671.
>> Fixes ticket #5866.
>>
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>>  libavformat/mxfdec.c | 281 ++++++++++++++++++++++++---------------------------
>>  1 file changed, 130 insertions(+), 151 deletions(-)
>
> causes a segfault:
>
> ==23735== Invalid read of size 8
> ==23735==    at 0x75A627: mxf_set_pts (mxfdec.c:3277)
> ==23735==    by 0x75ACAD: mxf_read_packet_old (mxfdec.c:3396)
> ==23735==    by 0x7E099D: ff_read_packet (utils.c:856)
> ==23735==    by 0x7E39FF: read_frame_internal (utils.c:1581)
> ==23735==    by 0x7EB82B: avformat_find_stream_info (utils.c:3773)
> ==23735==    by 0x415534: open_input_file (ffmpeg_opt.c:1091)
> ==23735==    by 0x41EB11: open_files (ffmpeg_opt.c:3206)
> ==23735==    by 0x41ECA3: ffmpeg_parse_options (ffmpeg_opt.c:3246)
> ==23735==    by 0x43D1A3: main (ffmpeg.c:4832)
> ==23735==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
> ==23735==
> ==23735==
> ==23735== Process terminating with default action of signal 11 (SIGSEGV)
> ==23735==  Access not within mapped region at address 0x0
> ==23735==    at 0x75A627: mxf_set_pts (mxfdec.c:3277)
> ==23735==    by 0x75ACAD: mxf_read_packet_old (mxfdec.c:3396)
> ==23735==    by 0x7E099D: ff_read_packet (utils.c:856)
> ==23735==    by 0x7E39FF: read_frame_internal (utils.c:1581)
> ==23735==    by 0x7EB82B: avformat_find_stream_info (utils.c:3773)
> ==23735==    by 0x415534: open_input_file (ffmpeg_opt.c:1091)
> ==23735==    by 0x41EB11: open_files (ffmpeg_opt.c:3206)
> ==23735==    by 0x41ECA3: ffmpeg_parse_options (ffmpeg_opt.c:3246)
> ==23735==    by 0x43D1A3: main (ffmpeg.c:4832)
> ==23735==  If you believe this happened as a result of a stack
> ==23735==  overflow in your program's main thread (unlikely but
> ==23735==  possible), you can try to increase the size of the
> ==23735==  main thread stack using the --main-stacksize= flag.
> ==23735==  The main thread stack size used in this run was 8388608.

I don't see this. What is your command line?

Thanks,
Marton
Paul B Mahol June 12, 2018, 9:05 a.m.
On 6/12/18, Michael Niedermayer <michael@niedermayer.cc> wrote:
> On Sun, Jun 10, 2018 at 12:36:47PM +0200, Marton Balint wrote:
>> Also use common code with opAtom.
>>
>> Fixes ticket #2776.
>> Partially fixes ticket #5671.
>> Fixes ticket #5866.
>>
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>>  libavformat/mxfdec.c | 281
>> ++++++++++++++++++++++++---------------------------
>>  1 file changed, 130 insertions(+), 151 deletions(-)
>
> causes a segfault:

Looks like some old habits never disappear.
Michael Niedermayer June 12, 2018, 6:17 p.m.
On Tue, Jun 12, 2018 at 10:47:24AM +0200, Marton Balint wrote:
> 
> 
> On Tue, 12 Jun 2018, Michael Niedermayer wrote:
> 
> >On Sun, Jun 10, 2018 at 12:36:47PM +0200, Marton Balint wrote:
> >>Also use common code with opAtom.
> >>
> >>Fixes ticket #2776.
> >>Partially fixes ticket #5671.
> >>Fixes ticket #5866.
> >>
> >>Signed-off-by: Marton Balint <cus@passwd.hu>
> >>---
> >> libavformat/mxfdec.c | 281 ++++++++++++++++++++++++---------------------------
> >> 1 file changed, 130 insertions(+), 151 deletions(-)
> >
> >causes a segfault:
> >
> >==23735== Invalid read of size 8
> >==23735==    at 0x75A627: mxf_set_pts (mxfdec.c:3277)
> >==23735==    by 0x75ACAD: mxf_read_packet_old (mxfdec.c:3396)
> >==23735==    by 0x7E099D: ff_read_packet (utils.c:856)
> >==23735==    by 0x7E39FF: read_frame_internal (utils.c:1581)
> >==23735==    by 0x7EB82B: avformat_find_stream_info (utils.c:3773)
> >==23735==    by 0x415534: open_input_file (ffmpeg_opt.c:1091)
> >==23735==    by 0x41EB11: open_files (ffmpeg_opt.c:3206)
> >==23735==    by 0x41ECA3: ffmpeg_parse_options (ffmpeg_opt.c:3246)
> >==23735==    by 0x43D1A3: main (ffmpeg.c:4832)
> >==23735==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
> >==23735==
> >==23735==
> >==23735== Process terminating with default action of signal 11 (SIGSEGV)
> >==23735==  Access not within mapped region at address 0x0
> >==23735==    at 0x75A627: mxf_set_pts (mxfdec.c:3277)
> >==23735==    by 0x75ACAD: mxf_read_packet_old (mxfdec.c:3396)
> >==23735==    by 0x7E099D: ff_read_packet (utils.c:856)
> >==23735==    by 0x7E39FF: read_frame_internal (utils.c:1581)
> >==23735==    by 0x7EB82B: avformat_find_stream_info (utils.c:3773)
> >==23735==    by 0x415534: open_input_file (ffmpeg_opt.c:1091)
> >==23735==    by 0x41EB11: open_files (ffmpeg_opt.c:3206)
> >==23735==    by 0x41ECA3: ffmpeg_parse_options (ffmpeg_opt.c:3246)
> >==23735==    by 0x43D1A3: main (ffmpeg.c:4832)
> >==23735==  If you believe this happened as a result of a stack
> >==23735==  overflow in your program's main thread (unlikely but
> >==23735==  possible), you can try to increase the size of the
> >==23735==  main thread stack using the --main-stacksize= flag.
> >==23735==  The main thread stack size used in this run was 8388608.
> 
> I don't see this. What is your command line?

testcase sent privatly

thx

[...]
Tomas Härdin June 13, 2018, 3:23 p.m.
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@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?

Can't really digest the rest of this patch

/Tomas
Marton Balint June 13, 2018, 8:11 p.m.
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@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.

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.

>
> Can't really digest the rest of this patch

Yeah, it is quite complex, I don't see an easy way to split it, sorry.

Regards,
Marton
Tomas Härdin June 14, 2018, 12:23 p.m.
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@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

> 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.

/Tomas
Marton Balint June 14, 2018, 9:09 p.m.
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@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.

Regards,
Marton
Marton Balint June 24, 2018, 7:01 p.m.
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@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

Patch hide | download patch | download mbox

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index 8054e051cf..d6d1628edb 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -278,13 +278,11 @@  typedef struct MXFContext {
     int local_tags_count;
     uint64_t footer_partition;
     KLVPacket current_klv_data;
-    int current_klv_index;
     int run_in;
     MXFPartition *current_partition;
     int parsing_backward;
     int64_t last_forward_tell;
     int last_forward_partition;
-    int64_t current_edit_unit;
     int nb_index_tables;
     MXFIndexTable *index_tables;
 } MXFContext;
@@ -2421,7 +2419,7 @@  static int mxf_parse_structural_metadata(MXFContext *mxf)
             if (ret < 0)
                 return ret;
         }
-        if (st->codecpar->codec_type != AVMEDIA_TYPE_DATA && (*essence_container_ul)[15] > 0x01) {
+        if (st->codecpar->codec_type != AVMEDIA_TYPE_DATA && source_track->wrapping != FrameWrapped) {
             /* TODO: decode timestamps */
             st->need_parsing = AVSTREAM_PARSE_TIMESTAMPS;
         }
@@ -2861,21 +2859,6 @@  static int is_pcm(enum AVCodecID codec_id)
     return codec_id >= AV_CODEC_ID_PCM_S16LE && codec_id < AV_CODEC_ID_PCM_S24DAUD;
 }
 
-static AVStream* mxf_get_opatom_stream(MXFContext *mxf)
-{
-    int i;
-
-    if (mxf->op != OPAtom)
-        return NULL;
-
-    for (i = 0; i < mxf->fc->nb_streams; i++) {
-        if (mxf->fc->streams[i]->codecpar->codec_type == AVMEDIA_TYPE_DATA)
-            continue;
-        return mxf->fc->streams[i];
-    }
-    return NULL;
-}
-
 static MXFIndexTable *mxf_find_index_table(MXFContext *mxf, int index_sid)
 {
     int i;
@@ -3175,47 +3158,6 @@  static int mxf_get_next_track_edit_unit(MXFContext *mxf, MXFTrack *track, int64_
     return 0;
 }
 
-/**
- * Sets mxf->current_edit_unit based on what offset we're currently at.
- * @return next_ofs if OK, <0 on error
- */
-static int64_t mxf_set_current_edit_unit(MXFContext *mxf, int64_t current_offset)
-{
-    int64_t last_ofs = -1, next_ofs = -1;
-    MXFIndexTable *t = &mxf->index_tables[0];
-
-    /* this is called from the OP1a demuxing logic, which means there
-     * may be no index tables */
-    if (mxf->nb_index_tables <= 0)
-        return -1;
-
-    /* find mxf->current_edit_unit so that the next edit unit starts ahead of current_offset */
-    while (mxf->current_edit_unit >= 0) {
-        if (mxf_edit_unit_absolute_offset(mxf, t, mxf->current_edit_unit + 1, NULL, &next_ofs, NULL, 0) < 0)
-            return -2;
-
-        if (next_ofs <= last_ofs) {
-            /* large next_ofs didn't change or current_edit_unit wrapped
-             * around this fixes the infinite loop on zzuf3.mxf */
-            av_log(mxf->fc, AV_LOG_ERROR,
-                   "next_ofs didn't change. not deriving packet timestamps\n");
-            return -1;
-        }
-
-        if (next_ofs > current_offset)
-            break;
-
-        last_ofs = next_ofs;
-        mxf->current_edit_unit++;
-    }
-
-    /* not checking mxf->current_edit_unit >= t->nb_ptses here since CBR files may lack IndexEntryArrays */
-    if (mxf->current_edit_unit < 0)
-        return -1;
-
-    return next_ofs;
-}
-
 static int64_t mxf_compute_sample_count(MXFContext *mxf, AVStream *st,
                                         int64_t edit_unit)
 {
@@ -3259,6 +3201,49 @@  static int64_t mxf_compute_sample_count(MXFContext *mxf, AVStream *st,
     return sample_count;
 }
 
+/**
+ * Make sure track->sample_count is correct based on what offset we're currently at.
+ * Also determine the next edit unit (or packet) offset.
+ * @return next_ofs if OK, <0 on error
+ */
+static int64_t mxf_set_current_edit_unit(MXFContext *mxf, AVStream *st, int64_t current_offset, int resync)
+{
+    int64_t next_ofs = -1;
+    MXFTrack *track = st->priv_data;
+    int64_t edit_unit = av_rescale_q(track->sample_count, st->time_base, av_inv_q(track->edit_rate));
+    int64_t new_edit_unit;
+    MXFIndexTable *t = mxf_find_index_table(mxf, track->index_sid);
+
+    if (!t || track->wrapping == UnknownWrapped)
+        return -1;
+
+    if (mxf_edit_unit_absolute_offset(mxf, t, edit_unit + track->edit_units_per_packet, NULL, &next_ofs, NULL, 0) < 0 &&
+        (next_ofs = mxf_essence_container_end(mxf, t->body_sid)) <= 0) {
+        av_log(mxf->fc, AV_LOG_ERROR, "unable to compute the size of the last packet\n");
+        return -1;
+    }
+
+    /* check if the next edit unit offset (next_ofs) starts ahead of current_offset */
+    if (next_ofs > current_offset)
+        return next_ofs;
+
+    if (!resync) {
+        av_log(mxf->fc, AV_LOG_ERROR, "cannot find current edit unit for stream %d, invalid index?\n", st->index);
+        return -1;
+    }
+
+    if (mxf_get_next_track_edit_unit(mxf, track, current_offset + 1, &new_edit_unit) < 0 || new_edit_unit <= 0) {
+        av_log(mxf->fc, AV_LOG_ERROR, "failed to find next track edit unit in stream %d\n", st->index);
+        return -1;
+    }
+
+    new_edit_unit--;
+    track->sample_count = mxf_compute_sample_count(mxf, st, new_edit_unit);
+    av_log(mxf->fc, AV_LOG_WARNING, "edit unit sync lost on stream %d, jumping from %"PRId64" to %"PRId64"\n", st->index, edit_unit, new_edit_unit);
+
+    return mxf_set_current_edit_unit(mxf, st, current_offset, 0);
+}
+
 static int mxf_set_audio_pts(MXFContext *mxf, AVCodecParameters *par,
                              AVPacket *pkt)
 {
@@ -3278,28 +3263,30 @@  static int mxf_set_audio_pts(MXFContext *mxf, AVCodecParameters *par,
     return 0;
 }
 
-static int mxf_set_pts(MXFContext *mxf, AVStream *st, AVPacket *pkt, int64_t next_ofs)
+static int mxf_set_pts(MXFContext *mxf, AVStream *st, AVPacket *pkt)
 {
     AVCodecParameters *par = st->codecpar;
     MXFTrack *track = st->priv_data;
 
-    if (par->codec_type == AVMEDIA_TYPE_VIDEO && (next_ofs >= 0 || next_ofs == -2 && st->duration == mxf->current_edit_unit + 1)) {
-        /* mxf->current_edit_unit good - see if we have an
-         * index table to derive timestamps from */
-        MXFIndexTable *t = &mxf->index_tables[0];
+    if (par->codec_type == AVMEDIA_TYPE_VIDEO) {
+        /* see if we have an index table to derive timestamps from */
+        MXFIndexTable *t = mxf_find_index_table(mxf, track->index_sid);
 
-        if (mxf->nb_index_tables >= 1 && mxf->current_edit_unit < t->nb_ptses) {
-            pkt->dts = mxf->current_edit_unit + t->first_dts;
-            pkt->pts = t->ptses[mxf->current_edit_unit];
+        if (t && track->sample_count < t->nb_ptses) {
+            pkt->dts = track->sample_count + t->first_dts;
+            pkt->pts = t->ptses[track->sample_count];
         } else if (track->intra_only) {
             /* intra-only -> PTS = EditUnit.
              * let utils.c figure out DTS since it can be < PTS if low_delay = 0 (Sony IMX30) */
-            pkt->pts = mxf->current_edit_unit;
+            pkt->pts = track->sample_count;
         }
+        track->sample_count++;
     } else if (par->codec_type == AVMEDIA_TYPE_AUDIO) {
         int ret = mxf_set_audio_pts(mxf, par, pkt);
         if (ret < 0)
             return ret;
+    } else if (track) {
+        track->sample_count++;
     }
     return 0;
 }
@@ -3310,7 +3297,17 @@  static int mxf_read_packet_old(AVFormatContext *s, AVPacket *pkt)
     MXFContext *mxf = s->priv_data;
     int ret;
 
-    while ((ret = klv_read_packet(&klv, s->pb)) == 0) {
+    while (1) {
+        int64_t max_data_size;
+        int64_t pos = avio_tell(s->pb);
+
+        if (pos < mxf->current_klv_data.next_klv - mxf->current_klv_data.length || pos >= mxf->current_klv_data.next_klv) {
+        mxf->current_klv_data = (KLVPacket){{0}};
+        ret = klv_read_packet(&klv, s->pb);
+        if (ret < 0)
+            break;
+        max_data_size = klv.length;
+        pos = klv.next_klv - klv.length;
         PRINT_KEY(s, "read packet", klv.key);
         av_log(s, AV_LOG_TRACE, "size %"PRIu64" offset %#"PRIx64"\n", klv.length, klv.offset);
         if (IS_KLV_KEY(klv.key, mxf_encrypted_triplet_key)) {
@@ -3321,6 +3318,10 @@  static int mxf_read_packet_old(AVFormatContext *s, AVPacket *pkt)
             }
             return 0;
         }
+        } else {
+            klv = mxf->current_klv_data;
+            max_data_size = klv.next_klv - pos;
+        }
         if (IS_KLV_KEY(klv.key, mxf_essence_element_key) ||
             IS_KLV_KEY(klv.key, mxf_canopus_essence_element_key) ||
             IS_KLV_KEY(klv.key, mxf_avid_essence_element_key)) {
@@ -3328,6 +3329,7 @@  static int mxf_read_packet_old(AVFormatContext *s, AVPacket *pkt)
             int index = mxf_get_stream_index(s, &klv, body_sid);
             int64_t next_ofs;
             AVStream *st;
+            MXFTrack *track;
 
             if (index < 0) {
                 av_log(s, AV_LOG_ERROR,
@@ -3337,21 +3339,39 @@  static int mxf_read_packet_old(AVFormatContext *s, AVPacket *pkt)
             }
 
             st = s->streams[index];
+            track = st->priv_data;
 
             if (s->streams[index]->discard == AVDISCARD_ALL)
                 goto skip;
 
-            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;
+                    if (klv.next_klv - klv.length == pos && max_data_size > max_packet_size) {
+                        st->need_parsing = AVSTREAM_PARSE_FULL;
+                        avpriv_request_sample(s, "Huge KLV without proper index in non-frame wrapped essence");
+                    }
+                    size = FFMIN(max_data_size, max_packet_size);
+                } else {
+                    if ((size = next_ofs - pos) <= 0) {
+                        av_log(s, AV_LOG_ERROR, "bad size: %"PRId64"\n", size);
+                        ret = AVERROR_INVALIDDATA;
+                        goto skip;
+                    }
+                    // We must not overread, because the next edit unit might be in another KLV
+                    if (size > max_data_size)
+                        size = max_data_size;
+                }
+
+                mxf->current_klv_data = klv;
+                klv.offset = pos;
+                klv.length = size;
+                klv.next_klv = klv.offset + klv.length;
             }
 
             /* check for 8 channels AES3 element */
@@ -3360,93 +3380,38 @@  static int mxf_read_packet_old(AVFormatContext *s, AVPacket *pkt)
                                               pkt, klv.length);
                 if (ret < 0) {
                     av_log(s, AV_LOG_ERROR, "error reading D-10 aes3 frame\n");
+                    mxf->current_klv_data = (KLVPacket){{0}};
                     return ret;
                 }
             } else {
                 ret = av_get_packet(s->pb, pkt, klv.length);
-                if (ret < 0)
+                if (ret < 0) {
+                    mxf->current_klv_data = (KLVPacket){{0}};
                     return ret;
+                }
             }
             pkt->stream_index = index;
             pkt->pos = klv.offset;
 
-            ret = mxf_set_pts(mxf, st, pkt, next_ofs);
-            if (ret < 0)
+            ret = mxf_set_pts(mxf, st, pkt);
+            if (ret < 0) {
+                mxf->current_klv_data = (KLVPacket){{0}};
                 return ret;
+            }
 
             /* seek for truncated packets */
             avio_seek(s->pb, klv.next_klv, SEEK_SET);
 
             return 0;
-        } else
+        } else {
         skip:
-            avio_skip(s->pb, klv.length);
+            avio_skip(s->pb, max_data_size);
+            mxf->current_klv_data = (KLVPacket){{0}};
+        }
     }
     return avio_feof(s->pb) ? AVERROR_EOF : ret;
 }
 
-static int mxf_read_packet(AVFormatContext *s, AVPacket *pkt)
-{
-    MXFContext *mxf = s->priv_data;
-    int ret, size;
-    int64_t ret64, pos, next_pos;
-    AVStream *st;
-    MXFIndexTable *t;
-    MXFTrack *track;
-    int edit_units;
-
-    if (mxf->op != OPAtom)
-        return mxf_read_packet_old(s, pkt);
-
-    // If we have no streams then we basically are at EOF
-    st = mxf_get_opatom_stream(mxf);
-    if (!st)
-        return AVERROR_EOF;
-
-    track = st->priv_data;
-
-    /* OPAtom - clip wrapped demuxing */
-    /* NOTE: mxf_read_header() makes sure nb_index_tables > 0 for OPAtom */
-    t = &mxf->index_tables[0];
-
-    if (mxf->current_edit_unit >= track->original_duration)
-        return AVERROR_EOF;
-
-    edit_units = FFMIN(track->edit_units_per_packet, track->original_duration - mxf->current_edit_unit);
-
-    if ((ret = mxf_edit_unit_absolute_offset(mxf, t, mxf->current_edit_unit, NULL, &pos, NULL, 1)) < 0)
-        return ret;
-
-    /* compute size by finding the next edit unit or the end of the essence container
-     * not pretty, but it works */
-    if ((ret = mxf_edit_unit_absolute_offset(mxf, t, mxf->current_edit_unit + edit_units, NULL, &next_pos, NULL, 0)) < 0 &&
-        (next_pos = mxf_essence_container_end(mxf, t->body_sid)) <= 0) {
-        av_log(s, AV_LOG_ERROR, "unable to compute the size of the last packet\n");
-        return AVERROR_INVALIDDATA;
-    }
-
-    if ((size = next_pos - pos) <= 0) {
-        av_log(s, AV_LOG_ERROR, "bad size: %i\n", size);
-        return AVERROR_INVALIDDATA;
-    }
-
-    if ((ret64 = avio_seek(s->pb, pos, SEEK_SET)) < 0)
-        return ret64;
-
-    if ((size = av_get_packet(s->pb, pkt, size)) < 0)
-        return size;
-
-    pkt->stream_index = st->index;
-
-    ret = mxf_set_pts(mxf, st, pkt, next_pos);
-    if (ret < 0)
-        return ret;
-
-    mxf->current_edit_unit += edit_units;
-
-    return 0;
-}
-
 static int mxf_read_close(AVFormatContext *s)
 {
     MXFContext *mxf = s->priv_data;
@@ -3536,8 +3501,10 @@  static int mxf_read_seek(AVFormatContext *s, int stream_index, int64_t sample_ti
             return seekpos;
 
         ff_update_cur_dts(s, st, sample_time);
-        mxf->current_edit_unit = sample_time;
+        mxf->current_klv_data = (KLVPacket){{0}};
     } else {
+        MXFPartition *partition;
+
         t = &mxf->index_tables[0];
         if (t->index_sid != source_track->index_sid) {
             /* If the first index table does not belong to the stream, then find a stream which does belong to the index table */
@@ -3580,11 +3547,23 @@  static int mxf_read_seek(AVFormatContext *s, int stream_index, int64_t sample_ti
             sample_time = FFMIN(sample_time, source_track->original_duration - 1);
         }
 
-        if ((ret = mxf_edit_unit_absolute_offset(mxf, t, sample_time, &sample_time, &seekpos, NULL, 1)) < 0)
+        if (source_track->wrapping == UnknownWrapped)
+            av_log(mxf->fc, AV_LOG_WARNING, "attempted seek in an UnknownWrapped essence\n");
+
+        if ((ret = mxf_edit_unit_absolute_offset(mxf, t, sample_time, &sample_time, &seekpos, &partition, 1)) < 0)
             return ret;
 
         ff_update_cur_dts(s, st, sample_time);
-        mxf->current_edit_unit = sample_time;
+        if (source_track->wrapping == ClipWrapped) {
+            KLVPacket klv = partition->first_essence_klv;
+            if (seekpos < klv.next_klv - klv.length || seekpos >= klv.next_klv) {
+                av_log(mxf->fc, AV_LOG_ERROR, "attempted seek out of clip wrapped KLV\n");
+                return AVERROR_INVALIDDATA;
+            }
+            mxf->current_klv_data = klv;
+        } else {
+            mxf->current_klv_data = (KLVPacket){{0}};
+        }
         avio_seek(s->pb, seekpos, SEEK_SET);
     }
 
@@ -3609,7 +3588,7 @@  AVInputFormat ff_mxf_demuxer = {
     .priv_data_size = sizeof(MXFContext),
     .read_probe     = mxf_probe,
     .read_header    = mxf_read_header,
-    .read_packet    = mxf_read_packet,
+    .read_packet    = mxf_read_packet_old,
     .read_close     = mxf_read_close,
     .read_seek      = mxf_read_seek,
 };