diff mbox

[FFmpeg-devel,4/6] avformat/mxfdec: remove check for NULL MXFTrack in mxf_set_pts

Message ID 20180531000536.11482-4-cus@passwd.hu
State Accepted
Commit d48fb904fa8189abef3108ddc36e575e805ee47e
Headers show

Commit Message

Marton Balint May 31, 2018, 12:05 a.m. UTC
It cannot happen for video streams.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/mxfdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Tomas Härdin June 5, 2018, 11:27 a.m. UTC | #1
tor 2018-05-31 klockan 02:05 +0200 skrev Marton Balint:
> It cannot happen for video streams.
> 
> > Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavformat/mxfdec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index e80ef62d57..cf1cd71987 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -3221,7 +3221,7 @@ static int mxf_set_pts(MXFContext *mxf, AVStream *st, AVPacket *pkt, int64_t nex
>          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];
> -        } else if (track && track->intra_only) {
> +        } else if (track->intra_only) {

I wonder what actually caused this check to be inserted. That is, what
could lead to track being NULL that is no longer the case?

/Tomas
Marton Balint June 5, 2018, 6:38 p.m. UTC | #2
On Tue, 5 Jun 2018, Tomas Härdin wrote:

> tor 2018-05-31 klockan 02:05 +0200 skrev Marton Balint:
>> It cannot happen for video streams.
>> 
>> > Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>>  libavformat/mxfdec.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
>> index e80ef62d57..cf1cd71987 100644
>> --- a/libavformat/mxfdec.c
>> +++ b/libavformat/mxfdec.c
>> @@ -3221,7 +3221,7 @@ static int mxf_set_pts(MXFContext *mxf, AVStream *st, AVPacket *pkt, int64_t nex
>>          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];
>> -        } else if (track && track->intra_only) {
>> +        } else if (track->intra_only) {
>
> I wonder what actually caused this check to be inserted. That is, what
> could lead to track being NULL that is no longer the case?

I don't know, I cc'd the original patch author about it. IMHO this was 
never needed because only streams which are added in 
mxf_add_metadata_stream have their priv_data (track) set to NULL, 
and those streams are always DATA.

Regards,
Marton
Tomas Härdin June 7, 2018, 8:52 a.m. UTC | #3
tis 2018-06-05 klockan 20:38 +0200 skrev Marton Balint:
> 
> On Tue, 5 Jun 2018, Tomas Härdin wrote:
> 
> > tor 2018-05-31 klockan 02:05 +0200 skrev Marton Balint:
> > > It cannot happen for video streams.
> > > 
> > > > Signed-off-by: Marton Balint <cus@passwd.hu>
> > > 
> > > ---
> > >  libavformat/mxfdec.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > > index e80ef62d57..cf1cd71987 100644
> > > --- a/libavformat/mxfdec.c
> > > +++ b/libavformat/mxfdec.c
> > > @@ -3221,7 +3221,7 @@ static int mxf_set_pts(MXFContext *mxf, AVStream *st, AVPacket *pkt, int64_t nex
> > >          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];
> > > -        } else if (track && track->intra_only) {
> > > +        } else if (track->intra_only) {
> > 
> > I wonder what actually caused this check to be inserted. That is, what
> > could lead to track being NULL that is no longer the case?
> 
> I don't know, I cc'd the original patch author about it. IMHO this was 
> never needed because only streams which are added in 
> mxf_add_metadata_stream have their priv_data (track) set to NULL, 
> and those streams are always DATA.

I can forsee someone in the future missing this. But I also don't like
cargo culting things that shouldn't be necessary. Grrr...

/Tomas
diff mbox

Patch

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index e80ef62d57..cf1cd71987 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -3221,7 +3221,7 @@  static int mxf_set_pts(MXFContext *mxf, AVStream *st, AVPacket *pkt, int64_t nex
         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];
-        } else if (track && track->intra_only) {
+        } 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;