Opened 5 years ago

Last modified 2 weeks ago

#8385 open defect

libavformat/aviobuf: A part of conditional expression is always true: whence != 2

Reported by: Balling Owned by:
Priority: normal Component: avformat
Version: git-master Keywords:
Cc: MasterQuestionable Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

If you look here https://github.com/FFmpeg/FFmpeg/blob/b741a84a15aed8afa01800dbc4b8b0e344e5d2da/libavformat/aviobuf.c#L297 you will see that on line 265 we would have already returned if whence were SEEK_END (2). It is not changed in between.

This change was done in commit: https://github.com/FFmpeg/FFmpeg/commit/7a6fe01f99cb95797ba59134f44b6666b1a5e792

whence != SEEK_END is always true, so short circuiting and force will not be checked.
Also see #8156 https://trac.ffmpeg.org/attachment/ticket/8156/project2019.tasks

Dunno how to fix it.

The code is like this ():

    if (whence != SEEK_CUR && whence != SEEK_SET)
        return AVERROR(EINVAL);

    if (whence == SEEK_CUR) {
        offset1 = pos + (s->buf_ptr - s->buffer);
        if (offset == 0)
            return offset1;
        if (offset > INT64_MAX - offset1)
            return AVERROR(EINVAL);
        offset += offset1;
    }
    if (offset < 0)
        return AVERROR(EINVAL);

    if (s->short_seek_get) {
        short_seek = s->short_seek_get(s->opaque);
        /* fallback to default short seek */
        if (short_seek <= 0)
            short_seek = s->short_seek_threshold;
    } else
        short_seek = s->short_seek_threshold;

    offset1 = offset - pos; // "offset1" is the relative offset from the beginning of s->buffer
    s->buf_ptr_max = FFMAX(s->buf_ptr_max, s->buf_ptr);
    if ((!s->direct || !s->seek) &&
        offset1 >= 0 && offset1 <= (s->write_flag ? s->buf_ptr_max - s->buffer : buffer_size)) {
        /* can do the seek inside the buffer */
        s->buf_ptr = s->buffer + offset1;
    } else if ((!(s->seekable & AVIO_SEEKABLE_NORMAL) ||
               offset1 <= buffer_size + short_seek) &&
               !s->write_flag && offset1 >= 0 &&
               (!s->direct || !s->seek) &&
              (whence != SEEK_END || force)) {

Change History (8)

comment:1 by Balling, 5 years ago

Status: newopen

Maybe you will fix this?))

comment:2 by Balling, 2 years ago

My patch is here. https://patchwork.ffmpeg.org/project/ffmpeg/patch/20210604062818.2099-1-val.zapod.vz@gmail.com/

THIS IS a fix for core API, and frankly speaking should be fixed, in fact it says there force does not work.

Last edited 2 weeks ago by Balling (previous) (diff)

comment:3 by Balling, 2 weeks ago

For IRC people...

This typos patch 41ed7ab45fc693f7d7fc35664c0233f4c32d69bb changed the documentation of AVSEEK_SIZE and AVSEEK_FORCE, it looks incorrect, other place documents that "optional AVSEEK_FORCE bit" is a thing, so I think "Oring this flag into "whence"" is more correct. It also says that Valid values are SEEK_SET, SEEK_CUR, SEEK_END (as in C library), AVSEEK_SIZE so those are just passed as is, it is only AVSEEK_FORCE that is just a bit and not an actual value.

James? Anyone?

comment:4 by MasterQuestionable, 2 weeks ago

Cc: MasterQuestionable added

͏    "OR this flag", for semantic clarity.
͏    .
͏    Derivatives of special words have to take special attention.
͏    Alike: https://trac.ffmpeg.org/ticket/11055#comment:40

͏    Also, how much relevance with this?
͏    https://trac.ffmpeg.org/ticket/11060

comment:5 by Balling, 2 weeks ago

See for yourself, this commit is wrong

https://github.com/FFmpeg/FFmpeg/commit/7a6fe01f99cb95797ba59134f44b6666b1a5e792

"Seeking forward in non-seekable media by discarding data, regardless of how far to seek. Won't SEEK_END unless forced though."

Since the commit is wrong, it will never SEEK_END, even if forced.

Last edited 2 weeks ago by Balling (previous) (diff)

comment:6 by MasterQuestionable, 2 weeks ago

͏    I did trace the aforementioned links.
͏    But too many things are too vaguely defined: meaning not easily fathomable.

͏    So I would like some more clear guidance from those who familiar with it.
͏    If necessary, I may rewrite the whole thing.
͏    (given clear specifications)

comment:7 by Balling, 2 weeks ago

Maybe the actual patch is that this line https://github.com/FFmpeg/FFmpeg/blob/b741a84a15aed8afa01800dbc4b8b0e344e5d2da/libavformat/aviobuf.c#L265

should add
whence != SEEK_END

That means later whence can be SEEK_END and will in fact work only if whence contained AVSEEK_FORCE flag (flag is stripoed from the whence and assigned to force variable).

Version 2, edited 2 weeks ago by Balling (previous) (next) (diff)
Note: See TracTickets for help on using tickets.