[FFmpeg-devel] [PATCH] Pass on position from ffmpeg to filters

[Víctor Paesa]

Hi,

On Sun, Jul 11, 2010 at 00:35, Michael Niedermayer wrote:
> On Sun, Jul 11, 2010 at 12:21:48AM +0200, V?ctor Paesa wrote:
>> Hi,
>>
>> On Sat, Jul 10, 2010 at 23:45, Michael Niedermayer wrote:
>> > On Sat, Jul 10, 2010 at 11:10:30PM +0200, V?ctor Paesa wrote:
>> >> Hi,
>> >> ffplay already passes on the file position to filters, but ffmpeg does not.
>> >>
>> >> I run the patched ffmpeg on the files generated by 'make test':
>> >> for i in tests/data/lavf/lavf.*
>> >> do
>> >> ? echo pos_$i
>> >> ? ./ffmpeg -debug 1 -vframes 25 -i $i -vf null -y a.avi 2>&1 </dev/null
>> >> done | grep pos
>> >>
>> >> For ASF, DV, FLV, GXF, MXF, SWF, y4m, the content of ist->st->cur_pkt.pos
>> >> does not look a reliable position.
>> >>
>> >> Thanks in advance for your review,
>> >> V?ctor
>> >
>> >> ?ffmpeg.c ? ? ? ? ? ? ? ? ?| ? ?5 ++++-
>> >> ?libavfilter/vsrc_buffer.c | ? ?5 ++++-
>> >> ?libavfilter/vsrc_buffer.h | ? ?2 +-
>> >> ?3 files changed, 9 insertions(+), 3 deletions(-)
>> >> ae2a17e652143de4ce4317537bcff1cbd4d3a040 ?ffmpeg.pos.1.diff
>> >> Index: ffmpeg.c
>> >> ===================================================================
>> >> --- ffmpeg.c ?(revision 24162)
>> >> +++ ffmpeg.c ?(working copy)
>> >> @@ -1648,7 +1648,10 @@
>> >> ? ? ? ? ? ? ?// add it to be filtered
>> >> ? ? ? ? ? ? ?av_vsrc_buffer_add_frame(ist->input_video_filter, &picture,
>> >> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ist->pts,
>> >> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ist->st->codec->sample_aspect_ratio);
>> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ist->st->codec->sample_aspect_ratio,
>> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ist->st->cur_pkt.pos);
>> >
>> > this looks wrong, pos from the AVPacket you get from the demuxer/parser
>> > should be used not some lavf internal one
>>
>> Pity. Would the next patch be OK to document that this is lavf internal?
>>
>> Index: libavformat/avformat.h
>> ===================================================================
>> --- libavformat/avformat.h ? ? ?(revision 24162)
>> +++ libavformat/avformat.h ? ? ?(working copy)
>> @@ -498,7 +498,7 @@
>>
>> ? ? ?AVMetadata *metadata;
>>
>> - ? ?/* av_read_frame() support */
>> + ? ?/* Only for av_read_frame() support. NOT PART OF PUBLIC API. */
>
> id say
> not supposed to be used by external applications, try to use something else
> if possible
>
> there may be situations where applications that do uncommon things need to
> peek into these, i think ive seen a case where cur_dts was the only way to
> get a hint of where one was exactly after a seek without demuxing packets

Hmm, we may need a native English to confer the proper degree of ambiguity :-)

What about this?

$ svn diff libavformat/avformat.h
Index: libavformat/avformat.h
===================================================================
--- libavformat/avformat.h      (revision 24162)
+++ libavformat/avformat.h      (working copy)
@@ -498,7 +498,8 @@

     AVMetadata *metadata;

-    /* av_read_frame() support */
+    /* Intended mostly for av_read_frame() support. Not supposed to
be used by */
+    /* external applications; try to use something else if at all
possible.    */
     const uint8_t *cur_ptr;
     int cur_len;
     AVPacket cur_pkt;

Regards,
V?ctor