[Ffmpeg-devel] Patch add function to read record time in dv video

[Michael Niedermayer]

Hi

On Sun, Sep 04, 2005 at 09:18:06AM -0600, Fred Rothganger wrote:
> Michael Niedermayer wrote:
> 
> >patch looks ok, roman, do you agree with that? IIRC we had some
> >disagreements in the past about how to pass this info around
> > 
> >
> 
>    Not to be too self-critical, but I think this is a pretty hacky way 
> to do it.
  
its simple and solves the problem, doing it correctly is hard and your
chances of getting a patch accepted in that area are near zero unless
you understand codecs, containers and ffmpeg well
  
  
> A nice way might be to put metadata in a name-value list 

yes


> attached to some appropriate structure, or perhaps have a standard 
> function for probing metadata.  For example:
> 
> AVInputFormat {
> ...
>    int (*metadata)(AVFormatContext *, char * name, void * value, int * 
> size);

what exactly is this supposed to do? and the name of the function is 
unaceptable


[...]

>  For 
> example, it could be a null terminated string or a time structure, etc.  
> "size" would be the size of the structure.

fine but i hope you are aware that you cannot store arbitrary structs in a 
portable way, it would need container and metadata type specific converters


> 
> The function would return metadata in the following priority order:
>    * Relative to the most recently decoded frame
>    * Relative to the container as a whole

rejected


> 
> If there are multiple streams and we want to be stream specific, a 
> metadata function could be added to AVCodec as well, or perhaps stream 
> could be one of the parameters of the function.

rejected


> 
> This approach could replace some of the metadata currently hard-coded 
> into AVFormatContext, such as title, author, copyright, etc.  For 
> writing such values to a stream, a metadata function can be added to 
> AVOutputFormat.

the way things like title, ... are handled currently is lame, and should be
changed

what you propose is missing many details and is not going to be accepted

the correct solution is to finish the AVOption code so we can enumerate/
set/get the fields of a struct, this could then be easily extended to an 
additional name/value list in the structure
this way we could access a field by its name independantly of where it is
actually stored, so we could change a member of the name/value list into
a normal C struct field without changing the code which accesses it

furthermore we could trivially store various things like a description
help text, max/min values or a function to check the validity of the value
...

[...]

-- 
Michael