[FFmpeg-devel] avformat/hlsenc: pull request for single file mode

[Nicolas Martyanoff]

On 2014-07-10 01:32, Anssi Hannula wrote:
> avformat/hlsenc: cleaning
> - looks good to me, and there is no maintainer so I guess this is OK
> - well, actually, maybe you could use more than one word in the subject :)
I'll change the title to something such as 'make the code easier to read' :)

> avformat/hlsenc: add single file mode
> - Adds HLSContext.ref_stream etc, shouldn't that be a separate patch?
I guess I could, but the code is incorrect without it. I added it when I
realized it would not work when a file has two video streams (or two audio
streams and no video streams).

If you think it's better to have two patches, the first one being incorrect, I
can split it.

> - HLSContext.file_idx is never read AFAICS
It is used to generate the TS filename in hls_create_file().

> avformat/hslenc: add a flag disabling the filename in segment names
> - hsl
Oops
> - should probably require single_file as well?
Indeed, I'll fix it.

> - see below
> 
> avformat/hlsenc: add an option to set the media filename
> - should probably check not used together with separated segments?
It also works with separated segments. For example:

    -hls_media_filename foo-%d.ts

Will generate foo-0.ts, foo-1.ts, foo-2.ts, etc.

Without this patch, separated files are named <playlist_base_filename>%d.ts.

> - see below
> 
> 
> "no_filename" for "do not use the name of the media file in segment
> names" were unclear enough that I had to take a look at the code what is
> going on.
> 
> For anyone else wondering (correct me if I'm wrong), currently:
> (1) The output file name from the user is used as the output media
> playlist filename.
> (2) The media file/segment file names are generated from the basename of
> the output playlist filename.
> (3) The media file/segments URLs in the media playlist get -hls_base_url
> prepended to them, so they are base_url + media filename.
This is indeed how it worked before my patches.

> These two patches would
> - add "no_filename" flag to only use hls_base_url for (3), allowing to
> select an arbitrary URL.
> - allow to select the file for (2)
Correct.

> I wonder if it would be clearer to have
> -hls_media_file - "the generated output media file"
>   (requires "single_file")
Not that as explained before, -hls_media_file is valid and used for
!single_file (i.e. individual files).

> -hls_media_url - "output media file url used in the playlist"
>   (requires "single_file" and !hls_base_url)
> 
> Or would it just be unclear to have both conflicting hls_media_url and
> hls_base_url?
I do not know. On the one hand, it seems redundant to have two different
options to specify the same thing, i.e. the URL used for segments. On the
other hand, with (HLS_SINGLE_FILE | HLS_NO_FILENAME), there is no "base URL",
only a "normal" URL.

It would be useful to have the point of view of someone using this muxer.


Thank you for taking the time to review my patches !

I'll update my patches tomorrow.

Regards,

-- 
Nicolas Martyanoff
http://wandrian.net
khaelin at gmail.com