[FFmpeg-devel] [PATCH] Make av_set_pts_info keep previous time base if new one is invalid.

[Ronald S. Bultje]

Hi,

On Sat, Feb 5, 2011 at 4:35 AM, Reimar D?ffinger
<Reimar.Doeffinger at gmx.de> wrote:
> ---
> ?libavformat/utils.c | ? 15 ++++++++-------
> ?1 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 4f51c26..c49d8ef 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -3742,16 +3742,17 @@ int ff_hex_to_data(uint8_t *data, const char *p)
> ?void av_set_pts_info(AVStream *s, int pts_wrap_bits,
> ? ? ? ? ? ? ? ? ? ? ?unsigned int pts_num, unsigned int pts_den)
> ?{
> - ? ?s->pts_wrap_bits = pts_wrap_bits;
> -
> - ? ?if(av_reduce(&s->time_base.num, &s->time_base.den, pts_num, pts_den, INT_MAX)){
> - ? ? ? ?if(s->time_base.num != pts_num)
> - ? ? ? ? ? ?av_log(NULL, AV_LOG_DEBUG, "st:%d removing common factor %d from timebase\n", s->index, pts_num/s->time_base.num);
> + ? ?AVRational new_tb;
> + ? ?if(av_reduce(&new_tb.num, &new_tb.den, pts_num, pts_den, INT_MAX)){
> + ? ? ? ?if(new_tb.num != pts_num)
> + ? ? ? ? ? ?av_log(NULL, AV_LOG_DEBUG, "st:%d removing common factor %d from timebase\n", s->index, pts_num/new_tb.num);
> ? ? }else
> ? ? ? ? av_log(NULL, AV_LOG_WARNING, "st:%d has too large timebase, reducing\n", s->index);
>
> - ? ?if(!s->time_base.num || !s->time_base.den)
> - ? ? ? ?s->time_base.num= s->time_base.den= 0;
> + ? ?if(new_tb.num <= 0 || new_tb.den <= 0)
> + ? ? ? ?return;
> + ? ?s->time_base = new_tb;
> + ? ?s->pts_wrap_bits = pts_wrap_bits;
> ?}
>
> ?int ff_url_join(char *str, int size, const char *proto,
> --
> 1.7.2.3

Not wanting to start a bikeshed, but I wonder if it's a good idea to
make av_set_pts_info() return an error code if it didn't set it, like
-1 or AVERROR(EINVAL) or whatever, and then perhaps we can even have
demuxers (optionally, this is not required because it has no security
implications) error out if av_set_pts_info() "fails".

At the very least it should be documented in the doxy that it can
"fail" and that it means tb remains unchanged.

Ronald