Re: [FFmpeg-devel] [PATCH 4/5] avformat/mxfdec: fix data_essence_descriptor matching logic

2018-05-28 Thread Tomas Härdin 
mån 2018-05-28 klockan 00:16 +0200 skrev Marton Balint:
> 
> On Sun, 27 May 2018, Tomas Härdin wrote:
> 
> > sön 2018-05-27 klockan 21:21 +0200 skrev Marton Balint:
> > > > Signed-off-by: Marton Balint 
> > > 
> > > ---
> > >  libavformat/mxfdec.c | 10 +-
> > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > > index bd46572e48..a62021b0d7 100644
> > > --- a/libavformat/mxfdec.c
> > > +++ b/libavformat/mxfdec.c
> > > @@ -2354,12 +2354,12 @@ static int
> > > mxf_parse_structural_metadata(MXFContext *mxf)
> > >  st->need_parsing = AVSTREAM_PARSE_FULL;
> > >  }
> > >  } else if (st->codecpar->codec_type ==
> > > AVMEDIA_TYPE_DATA) {
> > > -int codec_id =
> > > mxf_get_codec_ul(mxf_data_essence_container_uls,
> > > -essence_container_ul
> > > )->id;
> > > -if (codec_id >= 0 &&
> > > -codec_id <
> > > FF_ARRAY_ELEMS(mxf_data_essence_descriptor)) {
> > > +int index;
> > > +container_ul =
> > > mxf_get_codec_ul(mxf_data_essence_container_uls,
> > > essence_container_ul);
> > > +index = container_ul -
> > > mxf_data_essence_container_uls;
> > 
> > Nice use of C peculiarities (:
> 
> The other way is to add a char* to the MXFCodecUL struct, but I
> thought 
> that would be overkill for such a tiny use case.
> 
> > 
> > > +if (index <
> > > FF_ARRAY_ELEMS(mxf_data_essence_descriptor)) {
> > 
> > index can never be <0? Say if container_ul == NULL...
> 
> No, because mxf_get_codec_ul never returns NULL.

Alright, looks OK then. Still irked by the lack of static analysis.
Maybe should give Frama-C a go in some module one day

/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 4/5] avformat/mxfdec: fix data_essence_descriptor matching logic

2018-05-27 Thread Marton Balint 



On Sun, 27 May 2018, Tomas Härdin wrote:


sön 2018-05-27 klockan 21:21 +0200 skrev Marton Balint:

Signed-off-by: Marton Balint 

---
 libavformat/mxfdec.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index bd46572e48..a62021b0d7 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -2354,12 +2354,12 @@ static int mxf_parse_structural_metadata(MXFContext 
*mxf)
 st->need_parsing = AVSTREAM_PARSE_FULL;
 }
 } else if (st->codecpar->codec_type == AVMEDIA_TYPE_DATA) {
-int codec_id = mxf_get_codec_ul(mxf_data_essence_container_uls,
-essence_container_ul)->id;
-if (codec_id >= 0 &&
-codec_id < FF_ARRAY_ELEMS(mxf_data_essence_descriptor)) {
+int index;
+container_ul = mxf_get_codec_ul(mxf_data_essence_container_uls, 
essence_container_ul);
+index = container_ul - mxf_data_essence_container_uls;


Nice use of C peculiarities (:


The other way is to add a char* to the MXFCodecUL struct, but I thought 
that would be overkill for such a tiny use case.





+if (index < FF_ARRAY_ELEMS(mxf_data_essence_descriptor)) {


index can never be <0? Say if container_ul == NULL...


No, because mxf_get_codec_ul never returns NULL.

Regards,
Marton
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 4/5] avformat/mxfdec: fix data_essence_descriptor matching logic

2018-05-27 Thread Tomas Härdin 
sön 2018-05-27 klockan 21:21 +0200 skrev Marton Balint:
> > Signed-off-by: Marton Balint 
> ---
>  libavformat/mxfdec.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index bd46572e48..a62021b0d7 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -2354,12 +2354,12 @@ static int mxf_parse_structural_metadata(MXFContext 
> *mxf)
>  st->need_parsing = AVSTREAM_PARSE_FULL;
>  }
>  } else if (st->codecpar->codec_type == AVMEDIA_TYPE_DATA) {
> -int codec_id = mxf_get_codec_ul(mxf_data_essence_container_uls,
> -essence_container_ul)->id;
> -if (codec_id >= 0 &&
> -codec_id < FF_ARRAY_ELEMS(mxf_data_essence_descriptor)) {
> +int index;
> +container_ul = mxf_get_codec_ul(mxf_data_essence_container_uls, 
> essence_container_ul);
> +index = container_ul - mxf_data_essence_container_uls;

Nice use of C peculiarities (:

> +if (index < FF_ARRAY_ELEMS(mxf_data_essence_descriptor)) {

index can never be <0? Say if container_ul == NULL...

/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 4/5] avformat/mxfdec: fix data_essence_descriptor matching logic

2018-05-27 Thread Marton Balint 
Signed-off-by: Marton Balint 
---
 libavformat/mxfdec.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index bd46572e48..a62021b0d7 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -2354,12 +2354,12 @@ static int mxf_parse_structural_metadata(MXFContext 
*mxf)
 st->need_parsing = AVSTREAM_PARSE_FULL;
 }
 } else if (st->codecpar->codec_type == AVMEDIA_TYPE_DATA) {
-int codec_id = mxf_get_codec_ul(mxf_data_essence_container_uls,
-essence_container_ul)->id;
-if (codec_id >= 0 &&
-codec_id < FF_ARRAY_ELEMS(mxf_data_essence_descriptor)) {
+int index;
+container_ul = mxf_get_codec_ul(mxf_data_essence_container_uls, 
essence_container_ul);
+index = container_ul - mxf_data_essence_container_uls;
+if (index < FF_ARRAY_ELEMS(mxf_data_essence_descriptor)) {
 av_dict_set(>metadata, "data_type",
-mxf_data_essence_descriptor[codec_id], 0);
+mxf_data_essence_descriptor[index], 0);
 }
 }
 if (descriptor->extradata) {
-- 
2.16.3

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel