[FFmpeg-devel] [PATCH] First shot at AVCHD seeking via new seeking API

[Michael Niedermayer]

On Sat, Jul 25, 2009 at 01:14:12PM +0200, Ivan Schreter wrote:
> Hi,
>
> Ivan Schreter wrote:
>> Hi Baptiste,
>> [...]
>> I see we have a header "internal.h" in lavf. So I moved the declaration of 
>> my new routine there. This is then definitely no public API.
>>
>> OK to commit now (patches attached)?
>>
> Since noone reviewed my patches for about three weeks, I reviewed them 
> myself and wrote an extended seek test, which tests whether seeked-to 
> position really fulfills the contract of avformat_seek_file. This did 
> reveal a small problem, which I fixed. BTW, other formats also have issues, 
> so I'll probably send a patch for seek test to the list, when time permits.
>
> I also simplified the code by removing handling of special cases, since it 
> just made it less clear. Here is the newest set of patches.
>
> Special case handling of seeking before first keyframe or after the last 
> keyframe is now handled only for old seeking API inside of mpegts.c 
> read_seek(). New seeking API will return -1 if trying to seek outside of 
> timestamp range.
>
> If noone objects, I'll commit them during the next week.

quick review below, also please donot commit this without someone having
reviewed and approved, seeking is quite important 


[...]
>  #endif /* AVFORMAT_INTERNAL_H */
> Index: libavformat/utils.c
> ===================================================================
> --- libavformat/utils.c	(revision 19507)
> +++ libavformat/utils.c	(working copy)
> @@ -1451,6 +1451,377 @@
>      return pos;
>  }
>  
> +/**
> + * Helper structure describing keyframe search state of one stream.
> + */
> +typedef struct {
> +    int active;         ///< Set to 1 for active streams, 0 for inactive.
> +

> +    int64_t epsilon;    ///< Rounding error based on time base.

Please perform exact calculations

[...]
> +        // Evaluate key frames with known TS (or any frames, if AVSEEK_FLAG_ANY set).
> +        if (pts != AV_NOPTS_VALUE && ((flg & PKT_FLAG_KEY) || (flags & AVSEEK_FLAG_ANY))) {
> +            // Rescale stream timestamp to AV_TIME_BASE.
> +            st = s->streams[idx];
> +            pts_tb = av_rescale(pts, AV_TIME_BASE * (int64_t)st->time_base.num, st->time_base.den);
> +            dts_tb = av_rescale(dts, AV_TIME_BASE * (int64_t)st->time_base.num, st->time_base.den);
> +

> +            if (flags & AVSEEK_FLAG_BYTE) {
> +                // Seek by byte position.
> +                if (pos <= timestamp) {
> +                    // Keyframe found before target position.
> +                    if (!sp->found_lo) {
> +                        // Found first keyframe lower than target position.
> +                        sp->found_lo = 1;
> +                        (*found_lo)++;
> +                        sp->pts_lo = pts_tb;
> +                        sp->dts_lo = dts_tb;
> +                        sp->pos_lo = pos;
> +                    } else if (sp->pos_lo < pos) {
> +                        // Found a better match (closer to target position).
> +                        sp->pts_lo = pts_tb;
> +                        sp->dts_lo = dts_tb;
> +                        sp->pos_lo = pos;
> +                    }

looks like some code can be factored out


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I wish the Xiph folks would stop pretending they've got something they
do not.  Somehow I fear this will remain a wish. -- M?ns Rullg?rd
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090727/5a93affb/attachment.pgp>