Re: [FFmpeg-devel] MXF : default fied dominance is TFF
On Wed, Sep 10, 2014 at 10:47:12PM +0200, Tomas Härdin wrote: > On Tue, 2014-09-09 at 12:37 +0200, Michael Niedermayer wrote: > > On Mon, Sep 08, 2014 at 12:17:14PM +, Gaullier Nicolas wrote: > > > I did not found an easy way to set up initialization values to properly > > > handle defaults but I am not a highly skilled developer, and maybe > > > someone will find how to implement this more elegantly. > > > They are also many other properties in mxf that are only optional, for > > > example component depth and horizontal/vertical subsampling factors that > > > are actually parsed, but as far from now it does not seem sufficiently > > > useful to distinguish between the initialization value '0' and 'not > > > present'. > > > In my opinion, in the solely case of the field dominance, when it is > > > found/set to '0', it seems interesting to fail/raise a warning at least, > > > but it is somewhat particular and should not involve a big code > > > refactoring to handle this. > > > > field_dominance could be initilaized to -1 (or some named identifer > > that is -1) > > and a case -1 be added to the switch() > > Initializing to the default value (1 = MXF_TFF) makes more sense. > For > any illegal value the demuxer should probably complain so the user can > complain at whoever wrote the bad muxer they're using. yes [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Awnsering whenever a program halts or runs forever is On a turing machine, in general impossible (turings halting problem). On any real computer, always possible as a real computer has a finite number of states N, and will either halt in less than N cycles or never halt. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] MXF : default fied dominance is TFF
On Tue, 2014-09-09 at 12:37 +0200, Michael Niedermayer wrote: > On Mon, Sep 08, 2014 at 12:17:14PM +, Gaullier Nicolas wrote: > > I did not found an easy way to set up initialization values to properly > > handle defaults but I am not a highly skilled developer, and maybe someone > > will find how to implement this more elegantly. > > They are also many other properties in mxf that are only optional, for > > example component depth and horizontal/vertical subsampling factors that > > are actually parsed, but as far from now it does not seem sufficiently > > useful to distinguish between the initialization value '0' and 'not > > present'. > > In my opinion, in the solely case of the field dominance, when it is > > found/set to '0', it seems interesting to fail/raise a warning at least, > > but it is somewhat particular and should not involve a big code refactoring > > to handle this. > > field_dominance could be initilaized to -1 (or some named identifer > that is -1) > and a case -1 be added to the switch() Initializing to the default value (1 = MXF_TFF) makes more sense. For any illegal value the demuxer should probably complain so the user can complain at whoever wrote the bad muxer they're using. /Tomas signature.asc Description: This is a digitally signed message part ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] MXF : default fied dominance is TFF
On Mon, Sep 08, 2014 at 12:17:14PM +, Gaullier Nicolas wrote: > I did not found an easy way to set up initialization values to properly > handle defaults but I am not a highly skilled developer, and maybe someone > will find how to implement this more elegantly. > They are also many other properties in mxf that are only optional, for > example component depth and horizontal/vertical subsampling factors that are > actually parsed, but as far from now it does not seem sufficiently useful to > distinguish between the initialization value '0' and 'not present'. > In my opinion, in the solely case of the field dominance, when it is > found/set to '0', it seems interesting to fail/raise a warning at least, but > it is somewhat particular and should not involve a big code refactoring to > handle this. field_dominance could be initilaized to -1 (or some named identifer that is -1) and a case -1 be added to the switch() [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If a bugfix only changes things apparently unrelated to the bug with no further explanation, that is a good sign that the bugfix is wrong. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] MXF : default fied dominance is TFF
On 2014-09-08 14:17, Gaullier Nicolas wrote: > [FFmpeg-devel] MXF : default fied dominance is TFF That should be *field* dominance. If you're going to submit another patch please correct the commit message. signature.asc Description: OpenPGP digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] MXF : default fied dominance is TFF
I did not found an easy way to set up initialization values to properly handle
defaults but I am not a highly skilled developer, and maybe someone will find
how to implement this more elegantly.
They are also many other properties in mxf that are only optional, for example
component depth and horizontal/vertical subsampling factors that are actually
parsed, but as far from now it does not seem sufficiently useful to distinguish
between the initialization value '0' and 'not present'.
In my opinion, in the solely case of the field dominance, when it is found/set
to '0', it seems interesting to fail/raise a warning at least, but it is
somewhat particular and should not involve a big code refactoring to handle
this.
Nicolas
-Message d'origine-
De : ffmpeg-devel-boun...@ffmpeg.org [mailto:ffmpeg-devel-boun...@ffmpeg.org]
De la part de Carl Eugen Hoyos
Envoyé : lundi 8 septembre 2014 12:15
À : ffmpeg-devel@ffmpeg.org
Objet : Re: [FFmpeg-devel] MXF : default fied dominance is TFF
Gaullier Nicolas arkena.com> writes:
> case 0x3212:
> descriptor->field_dominance = avio_r8(pb);
> +descriptor->field_dominance_present = 1;
Is it possible to instead initialize
descriptor->field_dominance to MXF_TFF?
> case SeparateFields:
> +if (!descriptor->field_dominance_present)
> +descriptor->field_dominance = MXF_TFF;
> switch (descriptor->field_dominance) {
> case MXF_TFF:
> st->codec->field_order = AV_FIELD_TT;
Doesn't this allow to remove the default case below?
Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] MXF : default fied dominance is TFF
Gaullier Nicolas arkena.com> writes:
> case 0x3212:
> descriptor->field_dominance = avio_r8(pb);
> +descriptor->field_dominance_present = 1;
Is it possible to instead initialize
descriptor->field_dominance to MXF_TFF?
> case SeparateFields:
> +if (!descriptor->field_dominance_present)
> +descriptor->field_dominance = MXF_TFF;
> switch (descriptor->field_dominance) {
> case MXF_TFF:
> st->codec->field_order = AV_FIELD_TT;
Doesn't this allow to remove the default case below?
Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] MXF : default fied dominance is TFF
The field dominance is an optional property in the MXF specifications. It shall
be assumed to be 1 (top field first) if not present.
Nicolas
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index 7a4633f..7517285 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -152,6 +152,7 @@ typedef struct {
#define MXF_TFF 1
#define MXF_BFF 2
int field_dominance;
+int field_dominance_present;
int channels;
int bits_per_sample;
unsigned int component_depth;
@@ -872,6 +873,7 @@ static int mxf_read_generic_descriptor(void *arg,
AVIOContext *pb, int tag, int
break;
case 0x3212:
descriptor->field_dominance = avio_r8(pb);
+descriptor->field_dominance_present = 1;
break;
case 0x3301:
descriptor->component_depth = avio_rb32(pb);
@@ -1595,6 +1597,8 @@ static int mxf_parse_structural_metadata(MXFContext *mxf)
case MixedFields:
break;
case SeparateFields:
+if (!descriptor->field_dominance_present)
+descriptor->field_dominance = MXF_TFF;
switch (descriptor->field_dominance) {
case MXF_TFF:
st->codec->field_order = AV_FIELD_TT;
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
