subject:"\[FFmpeg\-devel\] \[PATCH 1\/6\] cbs\: Fragment\/unit data is always reference counted"

Re: [FFmpeg-devel] [PATCH 1/6] cbs: Fragment/unit data is always reference counted

2018-05-01 Thread Mark Thompson

On 01/05/18 16:51, James Almer wrote:
> On 4/30/2018 8:26 PM, Mark Thompson wrote:
>> Make this clear in the documentation and add some asserts to ensure
>> that it is always true.
>> ---
>> As suggested earlier :)
>>
>>
>>  libavcodec/cbs.c | 18 --
>>  libavcodec/cbs.h | 10 ++
>>  2 files changed, 18 insertions(+), 10 deletions(-)
>>
>> diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
>> index d81b4e03f7..0d6ffe8824 100644
>> --- a/libavcodec/cbs.c
>> +++ b/libavcodec/cbs.c
>> @@ -140,26 +140,30 @@ static int 
>> cbs_read_fragment_content(CodedBitstreamContext *ctx,
>>  int err, i, j;
>>  
>>  for (i = 0; i < frag->nb_units; i++) {
>> +CodedBitstreamUnit *unit = >units[i];
>> +
>>  if (ctx->decompose_unit_types) {
>>  for (j = 0; j < ctx->nb_decompose_unit_types; j++) {
>> -if (ctx->decompose_unit_types[j] == frag->units[i].type)
>> +if (ctx->decompose_unit_types[j] == unit->type)
>>  break;
>>  }
>>  if (j >= ctx->nb_decompose_unit_types)
>>  continue;
>>  }
>>  
>> -av_buffer_unref(>units[i].content_ref);
>> -frag->units[i].content = NULL;
>> +av_buffer_unref(>content_ref);
>> +unit->content = NULL;
>> +
>> +av_assert0(unit->data && unit->data_ref);
>>  
>> -err = ctx->codec->read_unit(ctx, >units[i]);
>> +err = ctx->codec->read_unit(ctx, unit);
>>  if (err == AVERROR(ENOSYS)) {
>>  av_log(ctx->log_ctx, AV_LOG_VERBOSE,
>> "Decomposition unimplemented for unit %d "
>> -   "(type %"PRIu32").\n", i, frag->units[i].type);
>> +   "(type %"PRIu32").\n", i, unit->type);
>>  } else if (err < 0) {
>>  av_log(ctx->log_ctx, AV_LOG_ERROR, "Failed to read unit %d "
>> -   "(type %"PRIu32").\n", i, frag->units[i].type);
>> +   "(type %"PRIu32").\n", i, unit->type);
>>  return err;
>>  }
>>  }
>> @@ -277,6 +281,7 @@ int ff_cbs_write_fragment_data(CodedBitstreamContext 
>> *ctx,
>> "(type %"PRIu32").\n", i, unit->type);
>>  return err;
>>  }
>> +av_assert0(unit->data && unit->data_ref);
>>  }
>>  
>>  av_buffer_unref(>data_ref);
>> @@ -287,6 +292,7 @@ int ff_cbs_write_fragment_data(CodedBitstreamContext 
>> *ctx,
>>  av_log(ctx->log_ctx, AV_LOG_ERROR, "Failed to assemble 
>> fragment.\n");
>>  return err;
>>  }
>> +av_assert0(frag->data && frag->data_ref);
> 
> You can remove the assert in ff_cbs_write_packet() if you add this one.

Yes, that would be sensible - removed.

>>  
>>  return 0;
>>  }
>> diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
>> index 402eb39e00..487358afaf 100644
>> --- a/libavcodec/cbs.h
>> +++ b/libavcodec/cbs.h
>> @@ -84,8 +84,9 @@ typedef struct CodedBitstreamUnit {
>>   */
>>  size_t   data_bit_padding;
>>  /**
>> - * If data is reference counted, a reference to the buffer containing
>> - * data.  Null if data is not reference counted.
>> + * A reference to the buffer containing data.
>> + *
>> + * Must be set if data is not NULL.
>>   */
>>  AVBufferRef *data_ref;
>>  
>> @@ -130,8 +131,9 @@ typedef struct CodedBitstreamFragment {
>>   */
>>  size_t data_bit_padding;
>>  /**
>> - * If data is reference counted, a reference to the buffer containing
>> - * data.  Null if data is not reference counted.
>> + * A reference to the buffer containing data.
>> + *
>> + * Must be set if data is not NULL.
>>   */
>>  AVBufferRef *data_ref;
> 
> LGTM.

And applied.

Thanks,

- Mark
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/6] cbs: Fragment/unit data is always reference counted

2018-05-01 Thread James Almer

On 4/30/2018 8:26 PM, Mark Thompson wrote:
> Make this clear in the documentation and add some asserts to ensure
> that it is always true.
> ---
> As suggested earlier :)
> 
> 
>  libavcodec/cbs.c | 18 --
>  libavcodec/cbs.h | 10 ++
>  2 files changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
> index d81b4e03f7..0d6ffe8824 100644
> --- a/libavcodec/cbs.c
> +++ b/libavcodec/cbs.c
> @@ -140,26 +140,30 @@ static int 
> cbs_read_fragment_content(CodedBitstreamContext *ctx,
>  int err, i, j;
>  
>  for (i = 0; i < frag->nb_units; i++) {
> +CodedBitstreamUnit *unit = >units[i];
> +
>  if (ctx->decompose_unit_types) {
>  for (j = 0; j < ctx->nb_decompose_unit_types; j++) {
> -if (ctx->decompose_unit_types[j] == frag->units[i].type)
> +if (ctx->decompose_unit_types[j] == unit->type)
>  break;
>  }
>  if (j >= ctx->nb_decompose_unit_types)
>  continue;
>  }
>  
> -av_buffer_unref(>units[i].content_ref);
> -frag->units[i].content = NULL;
> +av_buffer_unref(>content_ref);
> +unit->content = NULL;
> +
> +av_assert0(unit->data && unit->data_ref);
>  
> -err = ctx->codec->read_unit(ctx, >units[i]);
> +err = ctx->codec->read_unit(ctx, unit);
>  if (err == AVERROR(ENOSYS)) {
>  av_log(ctx->log_ctx, AV_LOG_VERBOSE,
> "Decomposition unimplemented for unit %d "
> -   "(type %"PRIu32").\n", i, frag->units[i].type);
> +   "(type %"PRIu32").\n", i, unit->type);
>  } else if (err < 0) {
>  av_log(ctx->log_ctx, AV_LOG_ERROR, "Failed to read unit %d "
> -   "(type %"PRIu32").\n", i, frag->units[i].type);
> +   "(type %"PRIu32").\n", i, unit->type);
>  return err;
>  }
>  }
> @@ -277,6 +281,7 @@ int ff_cbs_write_fragment_data(CodedBitstreamContext *ctx,
> "(type %"PRIu32").\n", i, unit->type);
>  return err;
>  }
> +av_assert0(unit->data && unit->data_ref);
>  }
>  
>  av_buffer_unref(>data_ref);
> @@ -287,6 +292,7 @@ int ff_cbs_write_fragment_data(CodedBitstreamContext *ctx,
>  av_log(ctx->log_ctx, AV_LOG_ERROR, "Failed to assemble fragment.\n");
>  return err;
>  }
> +av_assert0(frag->data && frag->data_ref);

You can remove the assert in ff_cbs_write_packet() if you add this one.

>  
>  return 0;
>  }
> diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
> index 402eb39e00..487358afaf 100644
> --- a/libavcodec/cbs.h
> +++ b/libavcodec/cbs.h
> @@ -84,8 +84,9 @@ typedef struct CodedBitstreamUnit {
>   */
>  size_t   data_bit_padding;
>  /**
> - * If data is reference counted, a reference to the buffer containing
> - * data.  Null if data is not reference counted.
> + * A reference to the buffer containing data.
> + *
> + * Must be set if data is not NULL.
>   */
>  AVBufferRef *data_ref;
>  
> @@ -130,8 +131,9 @@ typedef struct CodedBitstreamFragment {
>   */
>  size_t data_bit_padding;
>  /**
> - * If data is reference counted, a reference to the buffer containing
> - * data.  Null if data is not reference counted.
> + * A reference to the buffer containing data.
> + *
> + * Must be set if data is not NULL.
>   */
>  AVBufferRef *data_ref;

LGTM.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 1/6] cbs: Fragment/unit data is always reference counted

2018-04-30 Thread Mark Thompson

Make this clear in the documentation and add some asserts to ensure
that it is always true.
---
As suggested earlier :)


 libavcodec/cbs.c | 18 --
 libavcodec/cbs.h | 10 ++
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
index d81b4e03f7..0d6ffe8824 100644
--- a/libavcodec/cbs.c
+++ b/libavcodec/cbs.c
@@ -140,26 +140,30 @@ static int 
cbs_read_fragment_content(CodedBitstreamContext *ctx,
 int err, i, j;
 
 for (i = 0; i < frag->nb_units; i++) {
+CodedBitstreamUnit *unit = >units[i];
+
 if (ctx->decompose_unit_types) {
 for (j = 0; j < ctx->nb_decompose_unit_types; j++) {
-if (ctx->decompose_unit_types[j] == frag->units[i].type)
+if (ctx->decompose_unit_types[j] == unit->type)
 break;
 }
 if (j >= ctx->nb_decompose_unit_types)
 continue;
 }
 
-av_buffer_unref(>units[i].content_ref);
-frag->units[i].content = NULL;
+av_buffer_unref(>content_ref);
+unit->content = NULL;
+
+av_assert0(unit->data && unit->data_ref);
 
-err = ctx->codec->read_unit(ctx, >units[i]);
+err = ctx->codec->read_unit(ctx, unit);
 if (err == AVERROR(ENOSYS)) {
 av_log(ctx->log_ctx, AV_LOG_VERBOSE,
"Decomposition unimplemented for unit %d "
-   "(type %"PRIu32").\n", i, frag->units[i].type);
+   "(type %"PRIu32").\n", i, unit->type);
 } else if (err < 0) {
 av_log(ctx->log_ctx, AV_LOG_ERROR, "Failed to read unit %d "
-   "(type %"PRIu32").\n", i, frag->units[i].type);
+   "(type %"PRIu32").\n", i, unit->type);
 return err;
 }
 }
@@ -277,6 +281,7 @@ int ff_cbs_write_fragment_data(CodedBitstreamContext *ctx,
"(type %"PRIu32").\n", i, unit->type);
 return err;
 }
+av_assert0(unit->data && unit->data_ref);
 }
 
 av_buffer_unref(>data_ref);
@@ -287,6 +292,7 @@ int ff_cbs_write_fragment_data(CodedBitstreamContext *ctx,
 av_log(ctx->log_ctx, AV_LOG_ERROR, "Failed to assemble fragment.\n");
 return err;
 }
+av_assert0(frag->data && frag->data_ref);
 
 return 0;
 }
diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
index 402eb39e00..487358afaf 100644
--- a/libavcodec/cbs.h
+++ b/libavcodec/cbs.h
@@ -84,8 +84,9 @@ typedef struct CodedBitstreamUnit {
  */
 size_t   data_bit_padding;
 /**
- * If data is reference counted, a reference to the buffer containing
- * data.  Null if data is not reference counted.
+ * A reference to the buffer containing data.
+ *
+ * Must be set if data is not NULL.
  */
 AVBufferRef *data_ref;
 
@@ -130,8 +131,9 @@ typedef struct CodedBitstreamFragment {
  */
 size_t data_bit_padding;
 /**
- * If data is reference counted, a reference to the buffer containing
- * data.  Null if data is not reference counted.
+ * A reference to the buffer containing data.
+ *
+ * Must be set if data is not NULL.
  */
 AVBufferRef *data_ref;
 
-- 
2.16.3

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/6] cbs: Fragment/unit data is always reference counted

Re: [FFmpeg-devel] [PATCH 1/6] cbs: Fragment/unit data is always reference counted

[FFmpeg-devel] [PATCH 1/6] cbs: Fragment/unit data is always reference counted

3 matches

Site Navigation

Mail list logo

Footer information