[FFmpeg-devel] [PATCH 1/3] avformat/mov: Increase support for common encryption.

Michael Niedermayer michael at niedermayer.cc
Thu Jan 25 03:46:05 EET 2018


On Wed, Jan 24, 2018 at 11:43:26AM -0800, Jacob Trimble wrote:
> On Mon, Jan 22, 2018 at 7:38 PM, Michael Niedermayer
> <michael at niedermayer.cc> wrote
> > [...]
> >> This removes support for saio/saiz atoms, but it was incorrect before.
> >> A follow-up change will add correct support for those.
> >
> > This removal should be done by a seperate patch if it is done.
> > diff has matched up the removed function with a added one making this
> > hard to read as is
> >
> 
> The problem is that the old code used the saiz atoms to parse the senc
> atom.  I split the patch up for readability, but the two patches need
> to be applied at the same time (or squashed) since the first breaks
> encrypted content.  But I can squash them again if it is preferable to
> not have a commit that intentionally breaks things.

I didnt investigate this deeply so there is likely a better option that
i miss but you could just remove the functions which become unused in a 
subsequent patch to prevent diff from messing the line matching up totally


> 
> >
> >>
> >> Signed-off-by: Jacob Trimble <modmaker at google.com>
> >> ---
> >>  libavformat/isom.h                     |  20 +-
> >>  libavformat/mov.c                      | 432 ++++++++++++++++++++++-----------
> >>  tests/fate/mov.mak                     |   8 +
> >>  tests/ref/fate/mov-frag-encrypted      |  57 +++++
> >>  tests/ref/fate/mov-tenc-only-encrypted |  57 +++++
> >>  5 files changed, 422 insertions(+), 152 deletions(-)
> >>  create mode 100644 tests/ref/fate/mov-frag-encrypted
> >>  create mode 100644 tests/ref/fate/mov-tenc-only-encrypted
> >
> > This depends on other patches you posted, this should be mentioned or
> > all patches should be in the same patchset in order
> >
> 
> This depends on
> http://ffmpeg.org/pipermail/ffmpeg-devel/2018-January/223754.html and
> the recently pushed change to libavutil/aes_ctr.  Should I add
> something to the commit message or is that enough?

If you post a new version, then there should be a mail or comment explaining
any dependancies on yet to be applied patches.
It should not be in the commit messages or commited changes ideally
This way people trying to test code dont need to guess what they need
to apply first before a patchset


[...]
> >> +static int get_current_encryption_info(MOVContext *c, MOVEncryptionIndex **encryption_index, MOVStreamContext **sc)
> >>  {
> >> +    MOVFragmentStreamInfo *frag_stream_info;
> >>      AVStream *st;
> >> -    MOVStreamContext *sc;
> >> -    size_t auxiliary_info_size;
> >> +    int i;
> >>
> >> -    if (c->decryption_key_len == 0 || c->fc->nb_streams < 1)
> >> -        return 0;
> >> +    frag_stream_info = get_current_frag_stream_info(&c->frag_index);
> >> +    if (frag_stream_info) {
> >> +        for (i = 0; i < c->fc->nb_streams; i++) {
> >> +            if (c->fc->streams[i]->id == frag_stream_info->id) {
> >> +              st = c->fc->streams[i];
> >> +              break;
> >> +            }
> >> +        }
> >
> > the indention is inconsistent here
> >
> 
> No it's not, it looks like it because the diff looks odd.  If you
> apply the patch, the indentation in this method is consistent.

Indention depth is 4 in mov*.c
the hunk seems to add lines with a depth of 2
I would be surprised if this is not in the file after applying the patch

personally i dont care about the depth that much but i know many other people
care so this needs to be fixed before this can be applied

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180125/8cbdf07f/attachment.sig>


More information about the ffmpeg-devel mailing list