[PATCH] avidec: Mark first frame as keyframe in case there are no keyframes.
This fixes seeking in filecopy.avi of Ticket504 Signed-off-by: Michael Niedermayer <michaelni@gmx.at> --- libavformat/avidec.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/libavformat/avidec.c b/libavformat/avidec.c index 704980c..226da27 100644 --- a/libavformat/avidec.c +++ b/libavformat/avidec.c @@ -1236,6 +1236,7 @@ static int avi_read_idx1(AVFormatContext *s, int size) unsigned int index, tag, flags, pos, len, first_packet = 1; unsigned last_pos= -1; int64_t idx1_pos, first_packet_pos = 0, data_offset = 0; + int anykey = 0; nb_index_entries = size / 16; if (nb_index_entries <= 0) @@ -1283,6 +1284,14 @@ static int avi_read_idx1(AVFormatContext *s, int size) av_add_index_entry(st, pos, ast->cum_len, len, 0, (flags&AVIIF_INDEX) ? AVINDEX_KEYFRAME : 0); ast->cum_len += get_duration(ast, len); last_pos= pos; + anykey |= flags&AVIIF_INDEX; + } + if (!anykey) { + for (index = 0; index < s->nb_streams; index++) { + st = s->streams[index]; + if (st->nb_index_entries) + st->index_entries[0].flags |= AVINDEX_KEYFRAME; + } } return 0; } -- 1.7.5.4
On 30 Jan 2012, at 23:58, Michael Niedermayer <michaelni@gmx.at> wrote:
This fixes seeking in filecopy.avi of Ticket504
This doesn't seem right to me to handle at demuxer level. For example for an auto-generated index the demuxer will not even be able to do such a hack. It seems more sensible for me for the seeking function to validate the index and have some fallback behaviour if it does not make sense.
On Tue, Jan 31, 2012 at 09:19:36AM +0100, Reimar Döffinger wrote:
On 30 Jan 2012, at 23:58, Michael Niedermayer <michaelni@gmx.at> wrote:
This fixes seeking in filecopy.avi of Ticket504
This doesn't seem right to me to handle at demuxer level. For example for an auto-generated index the demuxer will not even be able to do such a hack. It seems more sensible for me for the seeking function to validate the index and have some fallback behaviour if it does not make sense.
well, that would make seeking O(n) while its O(log n) ATM, we at least have to validate during adding index entries and iam not sure if doing this for non avi makes sense (cut mpg surely can start with non keyframes) [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB When you are offended at any man's fault, turn to yourself and study your own failings. Then you will forget your anger. -- Epictetus
This fixes seeking in filecopy.avi of Ticket504
This doesn't seem right to me to handle at demuxer level. For example for an auto-generated index the demuxer will not even be able to do such a hack. It seems more sensible for me for the seeking function to validate the index and have some fallback behaviour if it does not make sense.
well, that would make seeking O(n) while its O(log n) ATM, we at least have to validate during adding index entries
and iam not sure if doing this for non avi makes sense (cut mpg surely can start with non keyframes)
This might make sense: If the decoder->read_seek function has failed, go ahead in av_seek_frame after read_seek has failed and seek to the byte position of the first of the index_entries if there are any. If there are no index_entries, then seek to the byte position of whatever the first packet position would be for the stream which should be the same as the first of the index_entries if there are any. At this failure point, I don't think we care if the first frame is marked as a keyframe or not and the above seems reasonable. Initially there are some cases where there are no index_entries. Thats fine and normally these will be built as the file is read or when seeking. The thing about avformat_seek_file failing is you don't know why it failed internally. (log messages yes ,but these don't do you any good internally) It probably should only fail on more serious errors like a read error etc. This could also be controlled by another flag to state you want the above behavior. While handling this in av_seek_frame does not take care of av_index_search_timestamp failing, we pretty much know why that failed.
This fixes seeking in filecopy.avi of Ticket504
This doesn't seem right to me to handle at demuxer level. For example for an auto-generated index the demuxer will not even be able to do such a hack. It seems more sensible for me for the seeking function to validate the index and have some fallback behaviour if it does not make sense.
well, that would make seeking O(n) while its O(log n) ATM, we at least have to validate during adding index entries
and iam not sure if doing this for non avi makes sense (cut mpg surely can start with non keyframes)
This might make sense:
If the decoder->read_seek function has failed, go ahead in av_seek_frame after read_seek has failed and seek to the byte position of the first of the index_entries if there are any. If there are no index_entries, then seek to the byte position of whatever the first packet position would be for the stream which should be the same as the first of the index_entries if there are any.
At this failure point, I don't think we care if the first frame is marked as a keyframe or not and the above seems reasonable. Initially there are some cases where there are no index_entries. Thats fine and normally these will be built as the file is read or when seeking.
The thing about avformat_seek_file failing is you don't know why it failed internally. (log messages yes ,but these don't do you any good internally) It probably should only fail on more serious errors like a read error etc.
This could also be controlled by another flag to state you want the above behavior.
While handling this in av_seek_frame does not take care of av_index_search_timestamp failing, we pretty much know why that failed.
The other side of this is the first packet read should be reflected as a keyframe so not sure if handling this only in av_seek_frame is sufficient. Probably better to add a keyframe index point as the first index_entry.
On Tue, Jan 31, 2012 at 09:49:53PM +0100, Michael Niedermayer wrote:
On Tue, Jan 31, 2012 at 09:19:36AM +0100, Reimar Döffinger wrote:
On 30 Jan 2012, at 23:58, Michael Niedermayer <michaelni@gmx.at> wrote:
This fixes seeking in filecopy.avi of Ticket504
This doesn't seem right to me to handle at demuxer level. For example for an auto-generated index the demuxer will not even be able to do such a hack. It seems more sensible for me for the seeking function to validate the index and have some fallback behaviour if it does not make sense.
well, that would make seeking O(n) while its O(log n) ATM, we at least have to validate during adding index entries
and iam not sure if doing this for non avi makes sense (cut mpg surely can start with non keyframes)
Are there still objections against the patch ? Any volunteers to implement a different solution ? I would prefer to commit some fix. Which fix doesnt really matter to me ... [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Awnsering whenever a program halts or runs forever is On a turing machine, in general impossible (turings halting problem). On any real computer, always possible as a real computer has a finite number of states N, and will either halt in less than N cycles or never halt.
On Mon, Mar 12, 2012 at 06:43:06AM +0100, Michael Niedermayer wrote:
On Tue, Jan 31, 2012 at 09:49:53PM +0100, Michael Niedermayer wrote:
On Tue, Jan 31, 2012 at 09:19:36AM +0100, Reimar Döffinger wrote:
On 30 Jan 2012, at 23:58, Michael Niedermayer <michaelni@gmx.at> wrote:
This fixes seeking in filecopy.avi of Ticket504
This doesn't seem right to me to handle at demuxer level. For example for an auto-generated index the demuxer will not even be able to do such a hack. It seems more sensible for me for the seeking function to validate the index and have some fallback behaviour if it does not make sense.
well, that would make seeking O(n) while its O(log n) ATM, we at least have to validate during adding index entries
and iam not sure if doing this for non avi makes sense (cut mpg surely can start with non keyframes)
Are there still objections against the patch ? Any volunteers to implement a different solution ? I would prefer to commit some fix. Which fix doesnt really matter to me ...
applied original patch [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope
participants (3)
-
Don Moir -
Michael Niedermayer -
Reimar Döffinger