[FFmpeg-devel] [PATCH] avformat/hlsenc: added HLS encryption

[Michael Niedermayer]

On Fri, Jan 09, 2015 at 11:07:16PM +0000, Christian Suloway wrote:
> Resending attachment.
> 
> ________________________________________
> From: ffmpeg-devel-bounces at ffmpeg.org <ffmpeg-devel-bounces at ffmpeg.org> on behalf of Christian Suloway <csuloway at globaleagleent.com>
> Sent: Friday, January 9, 2015 5:02 PM
> To: FFmpeg development discussions and patches
> Subject: Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: added HLS encryption
> 
> New version of the patch attached to address your comments.
> Thanks,
> Christian
> 
> On Jan 7, 2015, at 2:17 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> 
> > Please add a testcase/example to either the documentation or
> > commit message
> 
> Example added to the commit message.
> 
> >> +    ff_get_line(pb, hls->key_uri, sizeof(hls->key_uri));
> >> +    hls->key_uri[strcspn(hls->key_uri, "\r\n")] = '\0';
> >> +
> >> +    ff_get_line(pb, hls->key_file, sizeof(hls->key_file));
> >> +    hls->key_file[strcspn(hls->key_file, "\r\n")] = '\0';
> >> +
> >> +    ff_get_line(pb, hls->iv_string, sizeof(hls->iv_string));
> >> +    hls->iv_string[strcspn(hls->iv_string, "\r\n")] = '\0';
> >
> > in what case are the strcspn() needed ?
> 
> Replaced with terminating character check/replace, and combined with error check.
> 
> >> +        if (!err)
> >> +            snprintf(iv_string, sizeof(iv_string), "%032llx", c->sequence);
> >
> > libavformat/hlsenc.c:407:13: warning: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 4 has type ‘int64_t’ [-Wformat]
> > libavformat/hlsenc.c:407:13: warning: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 4 has type ‘int64_t’ [-Wformat]
> 
> Replaced with PRIx64 macro.
> 
> >> +        filename_size = strlen(prefix) + strlen(oc->filename) + 1;
> >> +        filename = av_malloc(filename_size);
> >> +        if (!filename) {
> >> +            av_dict_free(&options);
> >> +            return AVERROR(ENOMEM);
> >> +        }
> >> +        av_strlcpy(filename, prefix, filename_size);
> >> +        av_strlcat(filename, oc->filename, filename_size);
> >
> > this looks like it can be simplified with av_asprintf()
> 
> Replaced with av_asprintf().
> 

>  doc/muxers.texi      |    9 +++
>  libavformat/hlsenc.c |  122 +++++++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 128 insertions(+), 3 deletions(-)
> 1850b49d6ee3a8a752f9c082ef4c985f64aa0bee  0001-avformat-hlsenc-added-HLS-encryption.patch
> From 8bcfb958269b6ca91f6d78b3f9d270e2eb255942 Mon Sep 17 00:00:00 2001
> From: Christian Suloway <csuloway at globaleagleent.com>
> Date: Fri, 9 Jan 2015 16:23:38 -0600
> Subject: [PATCH] avformat/hlsenc: added HLS encryption
> 
> Added HLS encryption with -hls_key_info_file <key_info_file> option. The
> first line of key_info_file specifies the key URI for the playlist. The
> second line specifies the path to the file containing the encryption
> key. An optional third line specifies an IV to use instead of the
> segment number. Changes to key_info_file will be reflected in segment
> encryption along with an entry in the playlist for the new key URI and
> IV.
> 
> Example:
> ffmpeg -f lavfi -i testsrc -c:v h264 -hls_key_info_file foo.keyinfo
> foo.m3u8
> 
> foo.keyinfo:
> ./foo.key
> foo.key
> 0123456789ABCDEF0123456789ABCDEF
> 
> foo.key is a 16-byte binary file

i created the 2 files and tried but got
error opening key file foo.key
apparently the code does fail with the relative path that is in the
example or rather i guess its relative to something else then the
file refereing to it
this should at least be documented

after moving the file i got
Error setting option encryption_iv to value foo.key
or
error reading key file ./foo.key

i got it working after a few tries but
please provide an example that works and documentation that describes
the feature in a way sufficient to use it without reading the code
ideally add a example to the documentation too

also the parser is fragile it requires to carefully place newlines
or it will fail, i think the previous strcspn() code was better if it
avoided that, i didnt realize this

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150110/2beca2bd/attachment.asc>