diff mbox

[FFmpeg-devel] avcodec/noise_bsf: Add keyframes option.

Message ID 1520369235-19635-1-git-send-email-joshua.allmann@gmail.com
State New
Headers show

Commit Message

Josh Allmann March 6, 2018, 8:47 p.m. UTC
---
 doc/bitstream_filters.texi |  5 +++++
 libavcodec/noise_bsf.c     | 12 ++++++++++++
 libavcodec/version.h       |  2 +-
 3 files changed, 18 insertions(+), 1 deletion(-)

Comments

Michael Niedermayer March 7, 2018, 6:03 p.m. UTC | #1
On Tue, Mar 06, 2018 at 12:47:15PM -0800, Josh Allmann wrote:
> ---
>  doc/bitstream_filters.texi |  5 +++++
>  libavcodec/noise_bsf.c     | 12 ++++++++++++
>  libavcodec/version.h       |  2 +-
>  3 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/bitstream_filters.texi b/doc/bitstream_filters.texi
> index cfd81fa12d..a9f17f32ec 100644
> --- a/doc/bitstream_filters.texi
> +++ b/doc/bitstream_filters.texi
> @@ -399,6 +399,11 @@ every byte is modified.
>  A numeral string, whose value is related to how often packets will be dropped.
>  Therefore, values below or equal to 0 are forbidden, and the lower the more
>  frequent packets will be dropped, with 1 meaning every packet is dropped.
> +@item keyframes
> +A numeral string, whose value indicates the number of keyframe packets that
> +will be dropped from the beginning of the stream. This option will not add
> +noise to the source data. If used with stream copy, then -copyinkf should
> +be added to the output options as well.
>  @end table
>  
>  The following example applies the modification to every byte but does not drop
> diff --git a/libavcodec/noise_bsf.c b/libavcodec/noise_bsf.c
> index 84b94032ad..363ea4fc71 100644
> --- a/libavcodec/noise_bsf.c
> +++ b/libavcodec/noise_bsf.c
> @@ -32,6 +32,7 @@ typedef struct NoiseContext {
>      const AVClass *class;
>      int amount;
>      int dropamount;
> +    int keyframes;
>      unsigned int state;
>  } NoiseContext;
>  
> @@ -49,6 +50,13 @@ static int noise(AVBSFContext *ctx, AVPacket *out)
>      if (ret < 0)
>          return ret;
>  
> +    if (s->keyframes > 0 && in->flags & AV_PKT_FLAG_KEY) {
> +      s->keyframes--;
> +      if (!s->keyframes) s->keyframes = -1;
> +      av_packet_free(&in);
> +      return AVERROR(EAGAIN);
> +    }

I think keyframe should work like dropamount, that is randomly.

a non random droping could be added, maybe by the user specifying in
a list what to drop.
That would be more powerfull and flexible but probably not much harder
to implement. also keeping keyframes and dropamount behave the same
is more consistent

also the indention depth mismatches the surrounding code


[...]
Josh Allmann March 7, 2018, 11:37 p.m. UTC | #2
On 7 March 2018 at 18:03, Michael Niedermayer <michael@niedermayer.cc> wrote:
> On Tue, Mar 06, 2018 at 12:47:15PM -0800, Josh Allmann wrote:
>> ---
>>  doc/bitstream_filters.texi |  5 +++++
>>  libavcodec/noise_bsf.c     | 12 ++++++++++++
>>  libavcodec/version.h       |  2 +-
>>  3 files changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/doc/bitstream_filters.texi b/doc/bitstream_filters.texi
>> index cfd81fa12d..a9f17f32ec 100644
>> --- a/doc/bitstream_filters.texi
>> +++ b/doc/bitstream_filters.texi
>> @@ -399,6 +399,11 @@ every byte is modified.
>>  A numeral string, whose value is related to how often packets will be dropped.
>>  Therefore, values below or equal to 0 are forbidden, and the lower the more
>>  frequent packets will be dropped, with 1 meaning every packet is dropped.
>> +@item keyframes
>> +A numeral string, whose value indicates the number of keyframe packets that
>> +will be dropped from the beginning of the stream. This option will not add
>> +noise to the source data. If used with stream copy, then -copyinkf should
>> +be added to the output options as well.
>>  @end table
>>
>>  The following example applies the modification to every byte but does not drop
>> diff --git a/libavcodec/noise_bsf.c b/libavcodec/noise_bsf.c
>> index 84b94032ad..363ea4fc71 100644
>> --- a/libavcodec/noise_bsf.c
>> +++ b/libavcodec/noise_bsf.c
>> @@ -32,6 +32,7 @@ typedef struct NoiseContext {
>>      const AVClass *class;
>>      int amount;
>>      int dropamount;
>> +    int keyframes;
>>      unsigned int state;
>>  } NoiseContext;
>>
>> @@ -49,6 +50,13 @@ static int noise(AVBSFContext *ctx, AVPacket *out)
>>      if (ret < 0)
>>          return ret;
>>
>> +    if (s->keyframes > 0 && in->flags & AV_PKT_FLAG_KEY) {
>> +      s->keyframes--;
>> +      if (!s->keyframes) s->keyframes = -1;
>> +      av_packet_free(&in);
>> +      return AVERROR(EAGAIN);
>> +    }
>

Thanks for the feedback.

> I think keyframe should work like dropamount, that is randomly.
>
> a non random droping could be added, maybe by the user specifying in
> a list what to drop.
> That would be more powerfull and flexible but probably not much harder

Something like this?

noise=keyframes=1,3,5,7

in order to drop the 1st, 3rd, 5th and 7th keyframes?

> also keeping keyframes and dropamount behave the same
> is more consistent
>

Do you mean more consistent with respect to the 'amount' param of added noise,
as opposed to the patch's behavior of skipping noise if the 'keyframes' param is
present?

> also the indention depth mismatches the surrounding code
>
>
> [...]
>
> --
> 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
>
Michael Niedermayer March 8, 2018, 12:11 a.m. UTC | #3
On Wed, Mar 07, 2018 at 11:37:00PM +0000, Josh Allmann wrote:
> On 7 March 2018 at 18:03, Michael Niedermayer <michael@niedermayer.cc> wrote:
> > On Tue, Mar 06, 2018 at 12:47:15PM -0800, Josh Allmann wrote:
> >> ---
> >>  doc/bitstream_filters.texi |  5 +++++
> >>  libavcodec/noise_bsf.c     | 12 ++++++++++++
> >>  libavcodec/version.h       |  2 +-
> >>  3 files changed, 18 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/doc/bitstream_filters.texi b/doc/bitstream_filters.texi
> >> index cfd81fa12d..a9f17f32ec 100644
> >> --- a/doc/bitstream_filters.texi
> >> +++ b/doc/bitstream_filters.texi
> >> @@ -399,6 +399,11 @@ every byte is modified.
> >>  A numeral string, whose value is related to how often packets will be dropped.
> >>  Therefore, values below or equal to 0 are forbidden, and the lower the more
> >>  frequent packets will be dropped, with 1 meaning every packet is dropped.
> >> +@item keyframes
> >> +A numeral string, whose value indicates the number of keyframe packets that
> >> +will be dropped from the beginning of the stream. This option will not add
> >> +noise to the source data. If used with stream copy, then -copyinkf should
> >> +be added to the output options as well.
> >>  @end table
> >>
> >>  The following example applies the modification to every byte but does not drop
> >> diff --git a/libavcodec/noise_bsf.c b/libavcodec/noise_bsf.c
> >> index 84b94032ad..363ea4fc71 100644
> >> --- a/libavcodec/noise_bsf.c
> >> +++ b/libavcodec/noise_bsf.c
> >> @@ -32,6 +32,7 @@ typedef struct NoiseContext {
> >>      const AVClass *class;
> >>      int amount;
> >>      int dropamount;
> >> +    int keyframes;
> >>      unsigned int state;
> >>  } NoiseContext;
> >>
> >> @@ -49,6 +50,13 @@ static int noise(AVBSFContext *ctx, AVPacket *out)
> >>      if (ret < 0)
> >>          return ret;
> >>
> >> +    if (s->keyframes > 0 && in->flags & AV_PKT_FLAG_KEY) {
> >> +      s->keyframes--;
> >> +      if (!s->keyframes) s->keyframes = -1;
> >> +      av_packet_free(&in);
> >> +      return AVERROR(EAGAIN);
> >> +    }
> >
> 
> Thanks for the feedback.
> 

> > I think keyframe should work like dropamount, that is randomly.
> >
> > a non random droping could be added, maybe by the user specifying in
> > a list what to drop.
> > That would be more powerfull and flexible but probably not much harder
> 
> Something like this?
> 
> noise=keyframes=1,3,5,7
> 
> in order to drop the 1st, 3rd, 5th and 7th keyframes?

yes something like this


> 
> > also keeping keyframes and dropamount behave the same
> > is more consistent
> >
> 
> Do you mean more consistent with respect to the 'amount' param of added noise,
> as opposed to the patch's behavior of skipping noise if the 'keyframes' param is
> present?

both. 
i meant the param but as you mention the skiping, that doesnt seem expected by a
user either so it likely would be confusing.

thx

[...]
diff mbox

Patch

diff --git a/doc/bitstream_filters.texi b/doc/bitstream_filters.texi
index cfd81fa12d..a9f17f32ec 100644
--- a/doc/bitstream_filters.texi
+++ b/doc/bitstream_filters.texi
@@ -399,6 +399,11 @@  every byte is modified.
 A numeral string, whose value is related to how often packets will be dropped.
 Therefore, values below or equal to 0 are forbidden, and the lower the more
 frequent packets will be dropped, with 1 meaning every packet is dropped.
+@item keyframes
+A numeral string, whose value indicates the number of keyframe packets that
+will be dropped from the beginning of the stream. This option will not add
+noise to the source data. If used with stream copy, then -copyinkf should
+be added to the output options as well.
 @end table
 
 The following example applies the modification to every byte but does not drop
diff --git a/libavcodec/noise_bsf.c b/libavcodec/noise_bsf.c
index 84b94032ad..363ea4fc71 100644
--- a/libavcodec/noise_bsf.c
+++ b/libavcodec/noise_bsf.c
@@ -32,6 +32,7 @@  typedef struct NoiseContext {
     const AVClass *class;
     int amount;
     int dropamount;
+    int keyframes;
     unsigned int state;
 } NoiseContext;
 
@@ -49,6 +50,13 @@  static int noise(AVBSFContext *ctx, AVPacket *out)
     if (ret < 0)
         return ret;
 
+    if (s->keyframes > 0 && in->flags & AV_PKT_FLAG_KEY) {
+      s->keyframes--;
+      if (!s->keyframes) s->keyframes = -1;
+      av_packet_free(&in);
+      return AVERROR(EAGAIN);
+    }
+
     if (s->dropamount > 0 && s->state % s->dropamount == 0) {
         s->state++;
         av_packet_free(&in);
@@ -65,6 +73,9 @@  static int noise(AVBSFContext *ctx, AVPacket *out)
 
     memcpy(out->data, in->data, in->size);
 
+    if (s->keyframes)
+      return ret;
+
     for (i = 0; i < out->size; i++) {
         s->state += out->data[i] + 1;
         if (s->state % amount == 0)
@@ -81,6 +92,7 @@  fail:
 static const AVOption options[] = {
     { "amount", NULL, OFFSET(amount), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX },
     { "dropamount", NULL, OFFSET(dropamount), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX },
+    { "keyframes", NULL, OFFSET(keyframes), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX },
     { NULL },
 };
 
diff --git a/libavcodec/version.h b/libavcodec/version.h
index ca18ce6e8b..1e84410d68 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -29,7 +29,7 @@ 
 
 #define LIBAVCODEC_VERSION_MAJOR  58
 #define LIBAVCODEC_VERSION_MINOR  13
-#define LIBAVCODEC_VERSION_MICRO 100
+#define LIBAVCODEC_VERSION_MICRO 101
 
 #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
                                                LIBAVCODEC_VERSION_MINOR, \