[FFmpeg-devel,08/12] avformat/mxfdec: add support for returning the partition for mxf_edit_unit_absolute_offset and mxf_absolute_bodysid_offset

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

Details

Message ID 20180610103650.10155-8-cus@passwd.hu
State Accepted
Commit e7b1a8371878a2e7bd318b81a7e2706d636efb58
Headers show

Commit Message

Marton Balint June 10, 2018, 10:36 a.m.
Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/mxfdec.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

Comments

Tomas Härdin June 13, 2018, 3:15 p.m.
sön 2018-06-10 klockan 12:36 +0200 skrev Marton Balint:
> > Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavformat/mxfdec.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index 0f4cc9c3a3..8054e051cf 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -1399,7 +1399,7 @@ static int mxf_get_sorted_table_segments(MXFContext *mxf, int *nb_sorted_segment
>  /**
>   * Computes the absolute file offset of the given essence container offset
>   */
> -static int mxf_absolute_bodysid_offset(MXFContext *mxf, int body_sid, int64_t offset, int64_t *offset_out)
> +static int mxf_absolute_bodysid_offset(MXFContext *mxf, int body_sid, int64_t offset, int64_t *offset_out, MXFPartition **partition_out)
>  {
>      MXFPartition *last_p = NULL;
>      int a, b, m, m0;
> @@ -1427,6 +1427,8 @@ static int mxf_absolute_bodysid_offset(MXFContext *mxf, int body_sid, int64_t of
>  
>      if (last_p && (!last_p->essence_length || last_p->essence_length > (offset - last_p->body_offset))) {
>          *offset_out = last_p->essence_offset + (offset - last_p->body_offset);
> +        if (partition_out)
> +            *partition_out = last_p;
>          return 0;
>      }
>  
> @@ -1461,7 +1463,7 @@ static int64_t mxf_essence_container_end(MXFContext *mxf, int body_sid)
>  }
>  
>  /* EditUnit -> absolute offset */
> -static int mxf_edit_unit_absolute_offset(MXFContext *mxf, MXFIndexTable *index_table, int64_t edit_unit, int64_t *edit_unit_out, int64_t *offset_out, int nag)
> +static int mxf_edit_unit_absolute_offset(MXFContext *mxf, MXFIndexTable *index_table, int64_t edit_unit, int64_t *edit_unit_out, int64_t *offset_out, MXFPartition **partition_out, int nag)

Putting the pointer last would give a more readable patch, plus it's
nicer if all output vars are on the end (just a style preference)

/Tomas
Marton Balint June 13, 2018, 8:16 p.m.
On Wed, 13 Jun 2018, Tomas Härdin wrote:

> sön 2018-06-10 klockan 12:36 +0200 skrev Marton Balint:
>> > Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>>  libavformat/mxfdec.c | 18 ++++++++++--------
>>  1 file changed, 10 insertions(+), 8 deletions(-)
>> 
>> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
>> index 0f4cc9c3a3..8054e051cf 100644
>> --- a/libavformat/mxfdec.c
>> +++ b/libavformat/mxfdec.c
>> @@ -1399,7 +1399,7 @@ static int mxf_get_sorted_table_segments(MXFContext *mxf, int *nb_sorted_segment
>>  /**
>>   * Computes the absolute file offset of the given essence container offset
>>   */
>> -static int mxf_absolute_bodysid_offset(MXFContext *mxf, int body_sid, int64_t offset, int64_t *offset_out)
>> +static int mxf_absolute_bodysid_offset(MXFContext *mxf, int body_sid, int64_t offset, int64_t *offset_out, MXFPartition **partition_out)
>>  {
>>      MXFPartition *last_p = NULL;
>>      int a, b, m, m0;
>> @@ -1427,6 +1427,8 @@ static int mxf_absolute_bodysid_offset(MXFContext *mxf, int body_sid, int64_t of
>>  
>>      if (last_p && (!last_p->essence_length || last_p->essence_length > (offset - last_p->body_offset))) {
>>          *offset_out = last_p->essence_offset + (offset - last_p->body_offset);
>> +        if (partition_out)
>> +            *partition_out = last_p;
>>          return 0;
>>      }
>>  
>> @@ -1461,7 +1463,7 @@ static int64_t mxf_essence_container_end(MXFContext *mxf, int body_sid)
>>  }
>>  
>>  /* EditUnit -> absolute offset */
>> -static int mxf_edit_unit_absolute_offset(MXFContext *mxf, MXFIndexTable *index_table, int64_t edit_unit, int64_t *edit_unit_out, int64_t *offset_out, int nag)
>> +static int mxf_edit_unit_absolute_offset(MXFContext *mxf, MXFIndexTable *index_table, int64_t edit_unit, int64_t *edit_unit_out, int64_t *offset_out, MXFPartition **partition_out, int nag)
>
> Putting the pointer last would give a more readable patch, plus it's
> nicer if all output vars are on the end (just a style preference)

This is your code as well ;), "nag" was already the last, so it would be 
strange to put *partition_out after "nag", but have *offset_out before 
"nag".

Regards,
Marton
Tomas Härdin June 14, 2018, 12:25 p.m.
ons 2018-06-13 klockan 22:16 +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:
> > > > Signed-off-by: Marton Balint <cus@passwd.hu>
> > > 
> > > ---
> > >  libavformat/mxfdec.c | 18 ++++++++++--------
> > >  1 file changed, 10 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > > index 0f4cc9c3a3..8054e051cf 100644
> > > --- a/libavformat/mxfdec.c
> > > +++ b/libavformat/mxfdec.c
> > > @@ -1399,7 +1399,7 @@ static int mxf_get_sorted_table_segments(MXFContext *mxf, int *nb_sorted_segment
> > >  /**
> > >   * Computes the absolute file offset of the given essence container offset
> > >   */
> > > -static int mxf_absolute_bodysid_offset(MXFContext *mxf, int body_sid, int64_t offset, int64_t *offset_out)
> > > +static int mxf_absolute_bodysid_offset(MXFContext *mxf, int body_sid, int64_t offset, int64_t *offset_out, MXFPartition **partition_out)
> > >  {
> > >      MXFPartition *last_p = NULL;
> > >      int a, b, m, m0;
> > > @@ -1427,6 +1427,8 @@ static int mxf_absolute_bodysid_offset(MXFContext *mxf, int body_sid, int64_t of
> > >  
> > >      if (last_p && (!last_p->essence_length || last_p->essence_length > (offset - last_p->body_offset))) {
> > >          *offset_out = last_p->essence_offset + (offset - last_p->body_offset);
> > > +        if (partition_out)
> > > +            *partition_out = last_p;
> > >          return 0;
> > >      }
> > >  
> > > @@ -1461,7 +1463,7 @@ static int64_t mxf_essence_container_end(MXFContext *mxf, int body_sid)
> > >  }
> > >  
> > >  /* EditUnit -> absolute offset */
> > > -static int mxf_edit_unit_absolute_offset(MXFContext *mxf, MXFIndexTable *index_table, int64_t edit_unit, int64_t *edit_unit_out, int64_t *offset_out, int nag)
> > > +static int mxf_edit_unit_absolute_offset(MXFContext *mxf, MXFIndexTable *index_table, int64_t edit_unit, int64_t *edit_unit_out, int64_t *offset_out, MXFPartition **partition_out, int nag)
> > 
> > Putting the pointer last would give a more readable patch, plus it's
> > nicer if all output vars are on the end (just a style preference)
> 
> This is your code as well ;), "nag" was already the last, so it would be 
> strange to put *partition_out after "nag", but have *offset_out before 
> "nag".

Yeah. Whatever you feel like is fine

/Tomas

Patch hide | download patch | download mbox

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index 0f4cc9c3a3..8054e051cf 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -1399,7 +1399,7 @@  static int mxf_get_sorted_table_segments(MXFContext *mxf, int *nb_sorted_segment
 /**
  * Computes the absolute file offset of the given essence container offset
  */
-static int mxf_absolute_bodysid_offset(MXFContext *mxf, int body_sid, int64_t offset, int64_t *offset_out)
+static int mxf_absolute_bodysid_offset(MXFContext *mxf, int body_sid, int64_t offset, int64_t *offset_out, MXFPartition **partition_out)
 {
     MXFPartition *last_p = NULL;
     int a, b, m, m0;
@@ -1427,6 +1427,8 @@  static int mxf_absolute_bodysid_offset(MXFContext *mxf, int body_sid, int64_t of
 
     if (last_p && (!last_p->essence_length || last_p->essence_length > (offset - last_p->body_offset))) {
         *offset_out = last_p->essence_offset + (offset - last_p->body_offset);
+        if (partition_out)
+            *partition_out = last_p;
         return 0;
     }
 
@@ -1461,7 +1463,7 @@  static int64_t mxf_essence_container_end(MXFContext *mxf, int body_sid)
 }
 
 /* EditUnit -> absolute offset */
-static int mxf_edit_unit_absolute_offset(MXFContext *mxf, MXFIndexTable *index_table, int64_t edit_unit, int64_t *edit_unit_out, int64_t *offset_out, int nag)
+static int mxf_edit_unit_absolute_offset(MXFContext *mxf, MXFIndexTable *index_table, int64_t edit_unit, int64_t *edit_unit_out, int64_t *offset_out, MXFPartition **partition_out, int nag)
 {
     int i;
     int64_t offset_temp = 0;
@@ -1496,7 +1498,7 @@  static int mxf_edit_unit_absolute_offset(MXFContext *mxf, MXFIndexTable *index_t
             if (edit_unit_out)
                 *edit_unit_out = edit_unit;
 
-            return mxf_absolute_bodysid_offset(mxf, index_table->body_sid, offset_temp, offset_out);
+            return mxf_absolute_bodysid_offset(mxf, index_table->body_sid, offset_temp, offset_out, partition_out);
         } else {
             /* EditUnitByteCount == 0 for VBR indexes, which is fine since they use explicit StreamOffsets */
             offset_temp += s->edit_unit_byte_count * s->index_duration;
@@ -3160,7 +3162,7 @@  static int mxf_get_next_track_edit_unit(MXFContext *mxf, MXFTrack *track, int64_
 
     while (b - a > 1) {
         m = (a + b) >> 1;
-        if (mxf_edit_unit_absolute_offset(mxf, t, m, NULL, &offset, 0) < 0)
+        if (mxf_edit_unit_absolute_offset(mxf, t, m, NULL, &offset, NULL, 0) < 0)
             return -1;
         if (offset < current_offset)
             a = m;
@@ -3189,7 +3191,7 @@  static int64_t mxf_set_current_edit_unit(MXFContext *mxf, int64_t current_offset
 
     /* 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, 0) < 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) {
@@ -3412,12 +3414,12 @@  static int mxf_read_packet(AVFormatContext *s, AVPacket *pkt)
 
     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, 1)) < 0)
+    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, 0)) < 0 &&
+    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;
@@ -3578,7 +3580,7 @@  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, 1)) < 0)
+        if ((ret = mxf_edit_unit_absolute_offset(mxf, t, sample_time, &sample_time, &seekpos, NULL, 1)) < 0)
             return ret;
 
         ff_update_cur_dts(s, st, sample_time);