mbox series

[FFmpeg-devel,0/2] fix for seeking in HLS with TS/FMP4 media

Message ID 20200424152042.29383-1-hello.vectronic@gmail.com
Headers show
Series fix for seeking in HLS with TS/FMP4 media | expand

Message

vectronic April 24, 2020, 3:20 p.m. UTC
I am resubmitting a patch which fixes the following two tickets:

https://trac.ffmpeg.org/ticket/7359
https://trac.ffmpeg.org/ticket/7485

The patch consists of 2 parts. The first is necessary to fix the case of HLS seeking when
the HLS package consists of ts files. The second fixes HLS seeking when the HLS package
consists of fmp4 files. Note that the first can be applied without the second, and HLS
seeking with fmp4 files will remain broken, but in a different manner. The first patch
works because it relies on correct behaviour implemented in mpegts.c, the second patch mirrors this
behaviour in mov.c.

The issues can be demonstrated as follows:

./ffmpeg -ss 3 -i http://vectronic.io/hls_seek_issue/ts/in.m3u8 -t 2 out.mp4
./ffmpeg -ss 3 -i http://vectronic.io/hls_seek_issue/fmp4/in.m3u8 -t 2 out.mp4

These both produce zero duration output files.

With the first patch applied, the ts example produces a 2 second output file as expected.
However the fmp4 example results in a conversion error.

With the second patch applied, both the ts and fmp4 examples produce 2 second output files as expected.

Patch 1: Change to logic in hls.c for seeking

After performing a rough seek in hls.c hls_read_seek() to the nearest segment, the logic for hls_read_packet() was to discard packets until a DTS after the accurate seek point is discovered. This was done even if AVSEEK_FLAG_ANY was not specified in the initial seek request. The patch changes this so that if AVSEEK_FLAG_ANY has not been specified, it will instead return the first keyframe discovered after the rough seek point.

Patch 2: Change to logic in mov.c for reading packets after an HLS seek

Within hls.c when hls_read_seek() is called it resets the stream position as follows:

/* Reset the pos, to let the mpegts demuxer know we've seeked. */
pls->pb.pos = 0;

There is support for this in the mpegts handle_packets() code to check if the position has been reset and clear out any PES packets:

if (avio_tell(s->pb) != ts->last_pos) {
      int i;
      av_log(ts->stream, AV_LOG_TRACE, "Skipping after seek\n");
      /* seek detected, flush pes buffer */

This behaviour needed to be mirrored in the mov demuxer. In mov.c it now detects if the pos has been reset in mov_read_packet(). It clears fragments and indexes and searches for the next root i.e. the next fragment via mov_switch_root().

vectronic (2):
  avformat hls fix to seek logic
  avformat mov fix to detect if stream position has been reset

 libavformat/hls.c |  8 +++++---
 libavformat/mov.c | 36 +++++++++++++++++++++++++++++++++---
 2 files changed, 38 insertions(+), 6 deletions(-)

Comments

Carl Eugen Hoyos April 25, 2020, 9:42 p.m. UTC | #1
Am Fr., 24. Apr. 2020 um 17:21 Uhr schrieb vectronic
<hello.vectronic@gmail.com>:
>
> I am resubmitting a patch which fixes the following two tickets:
>
> https://trac.ffmpeg.org/ticket/7359
> https://trac.ffmpeg.org/ticket/7485

Are you Nick Ryan / did you write the attached patches?

Carl Eugen
Josh Dekker April 29, 2020, 11:59 a.m. UTC | #2
On 25/04/2020 22:42, Carl Eugen Hoyos wrote:
> Am Fr., 24. Apr. 2020 um 17:21 Uhr schrieb vectronic
> <hello.vectronic@gmail.com>:
>>
>> I am resubmitting a patch which fixes the following two tickets:
>>

Do you not want to commit with your name rather than a handle here?

>> https://trac.ffmpeg.org/ticket/7359
>> https://trac.ffmpeg.org/ticket/7485
> 
> Are you Nick Ryan / did you write the attached patches?

The LinkedIn on his website suggests he is.