On Mon, Mar 5, 2018 at 12:22 PM, Jacob Trimble <modma...@google.com> wrote: > On Mon, Feb 12, 2018 at 9:35 AM, Jacob Trimble <modma...@google.com> wrote: >> On Tue, Jan 30, 2018 at 11:27 AM, Jacob Trimble <modma...@google.com> wrote: >>> On Wed, Jan 24, 2018 at 5:46 PM, Michael Niedermayer >>> <mich...@niedermayer.cc> wrote: >>>> On Wed, Jan 24, 2018 at 11:43:26AM -0800, Jacob Trimble wrote: >>>>> On Mon, Jan 22, 2018 at 7:38 PM, Michael Niedermayer >>>>> <mich...@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 >>>> >>> >>> Done. >>> >>>> >>>>> >>>>> > >>>>> >> >>>>> >> Signed-off-by: Jacob Trimble <modma...@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 >>> >>> Didn't see that. Fixed and did a grep for incorrect indentations. >>> >>>> >>>> [...] >>>> >>>> -- >>>> 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 >>>> >>>> _______________________________________________ >>>> ffmpeg-devel mailing list >>>> ffmpeg-devel@ffmpeg.org >>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>>> >> >> Ping. This depends on >> http://ffmpeg.org/pipermail/ffmpeg-devel/2018-January/223754.html. > > Ping again. I know this is low priority, but I would like to get > these merged soon.
Ping. Despite being almost 2 months old, these patches still apply cleanly. Please take a look. These have been in review for almost 3 months. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel