Re: [FFmpeg-devel] [PATCH 2/7] avcodec/golomb: Make emitting error message in get_ue_golomb optional
James Almer:
> On 7/14/2020 12:34 PM, Andreas Rheinhardt wrote:
>> This is designed for scenarios where the caller already checks that the
>> returned value is within a certain allowed range and returns an error
>> message if not.
>>
>> Signed-off-by: Andreas Rheinhardt
>> ---
>> libavcodec/golomb.h | 32
>> 1 file changed, 24 insertions(+), 8 deletions(-)
>>
>> diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
>> index 5bfcfe085f..63069f63e5 100644
>> --- a/libavcodec/golomb.h
>> +++ b/libavcodec/golomb.h
>> @@ -47,12 +47,7 @@ extern const uint8_t
>> ff_interleaved_ue_golomb_vlc_code[256];
>> extern const int8_t ff_interleaved_se_golomb_vlc_code[256];
>> extern const uint8_t ff_interleaved_dirac_golomb_vlc_code[256];
>>
>> -/**
>> - * Read an unsigned Exp-Golomb code in the range 0 to 8190.
>> - *
>> - * @returns the read value or a negative error code.
>> - */
>> -static inline int get_ue_golomb(GetBitContext *gb)
>> +static inline int get_ue_golomb_internal(GetBitContext *gb, int
>> emit_error_msg)
>> {
>> unsigned int buf;
>>
>> @@ -67,7 +62,8 @@ static inline int get_ue_golomb(GetBitContext *gb)
>> } else {
>> int log = 2 * av_log2(buf) - 31;
>> if (log < 0) {
>> -av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
>> +if (emit_error_msg)
>> +av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
>> return AVERROR_INVALIDDATA;
>> }
>> buf >>= log;
>> @@ -92,7 +88,8 @@ static inline int get_ue_golomb(GetBitContext *gb)
>> LAST_SKIP_BITS(re, gb, 32 - log);
>> CLOSE_READER(re, gb);
>> if (log < 7) {
>> -av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
>> +if (emit_error_msg)
>> +av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
>> return AVERROR_INVALIDDATA;
>> }
>> buf >>= log;
>> @@ -103,6 +100,25 @@ static inline int get_ue_golomb(GetBitContext *gb)
>> #endif
>> }
>>
>> +/**
>> + * Read an unsigned Exp-Golomb code in the range 0 to 8190.
>> + *
>> + * @returns the read value or a negative error code.
>> + */
>> +static inline int get_ue_golomb(GetBitContext *gb)
>> +{
>> +return get_ue_golomb_internal(gb, 1);
>> +}
>> +
>> +/**
>> + * Variant of get_ue_golomb that does not emit an error message
>> + * if the number is outside the permissible range.
>> + */
>> +static inline int get_ue_golomb2(GetBitContext *gb)
>
> Seeing there's precedent for functions where the number suffix relates
> to the range of values they can parse, i think using a 2 here could be
> confusing at first glance. I suggest to use something else.
>
> Is it worth keeping the error messages at all, for that matter? They
> don't even use a proper logging context, so the printed output is ugly
> and unidentifiable by itself.
>
I could live very well without the error messages. If there are no
objections, I will just remove them (and not add them in the cached
codepath).
- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/7] avcodec/golomb: Make emitting error message in get_ue_golomb optional
James Almer (12020-07-14): > Is it worth keeping the error messages at all, for that matter? They > don't even use a proper logging context, so the printed output is ugly > and unidentifiable by itself. Good point. We should probably try to eliminate all av_log(NULL) uses from the libraries, in fact. There are only ~300 of them, after all, that can be done. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/7] avcodec/golomb: Make emitting error message in get_ue_golomb optional
On 7/14/2020 12:34 PM, Andreas Rheinhardt wrote:
> This is designed for scenarios where the caller already checks that the
> returned value is within a certain allowed range and returns an error
> message if not.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> libavcodec/golomb.h | 32
> 1 file changed, 24 insertions(+), 8 deletions(-)
>
> diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
> index 5bfcfe085f..63069f63e5 100644
> --- a/libavcodec/golomb.h
> +++ b/libavcodec/golomb.h
> @@ -47,12 +47,7 @@ extern const uint8_t
> ff_interleaved_ue_golomb_vlc_code[256];
> extern const int8_t ff_interleaved_se_golomb_vlc_code[256];
> extern const uint8_t ff_interleaved_dirac_golomb_vlc_code[256];
>
> -/**
> - * Read an unsigned Exp-Golomb code in the range 0 to 8190.
> - *
> - * @returns the read value or a negative error code.
> - */
> -static inline int get_ue_golomb(GetBitContext *gb)
> +static inline int get_ue_golomb_internal(GetBitContext *gb, int
> emit_error_msg)
> {
> unsigned int buf;
>
> @@ -67,7 +62,8 @@ static inline int get_ue_golomb(GetBitContext *gb)
> } else {
> int log = 2 * av_log2(buf) - 31;
> if (log < 0) {
> -av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
> +if (emit_error_msg)
> +av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
> return AVERROR_INVALIDDATA;
> }
> buf >>= log;
> @@ -92,7 +88,8 @@ static inline int get_ue_golomb(GetBitContext *gb)
> LAST_SKIP_BITS(re, gb, 32 - log);
> CLOSE_READER(re, gb);
> if (log < 7) {
> -av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
> +if (emit_error_msg)
> +av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
> return AVERROR_INVALIDDATA;
> }
> buf >>= log;
> @@ -103,6 +100,25 @@ static inline int get_ue_golomb(GetBitContext *gb)
> #endif
> }
>
> +/**
> + * Read an unsigned Exp-Golomb code in the range 0 to 8190.
> + *
> + * @returns the read value or a negative error code.
> + */
> +static inline int get_ue_golomb(GetBitContext *gb)
> +{
> +return get_ue_golomb_internal(gb, 1);
> +}
> +
> +/**
> + * Variant of get_ue_golomb that does not emit an error message
> + * if the number is outside the permissible range.
> + */
> +static inline int get_ue_golomb2(GetBitContext *gb)
Seeing there's precedent for functions where the number suffix relates
to the range of values they can parse, i think using a 2 here could be
confusing at first glance. I suggest to use something else.
Is it worth keeping the error messages at all, for that matter? They
don't even use a proper logging context, so the printed output is ugly
and unidentifiable by itself.
> +{
> +return get_ue_golomb_internal(gb, 0);
> +}
> +
> /**
> * Read an unsigned Exp-Golomb code in the range 0 to UINT32_MAX-1.
> */
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/7] avcodec/golomb: Make emitting error message in get_ue_golomb optional
This is designed for scenarios where the caller already checks that the
returned value is within a certain allowed range and returns an error
message if not.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/golomb.h | 32
1 file changed, 24 insertions(+), 8 deletions(-)
diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
index 5bfcfe085f..63069f63e5 100644
--- a/libavcodec/golomb.h
+++ b/libavcodec/golomb.h
@@ -47,12 +47,7 @@ extern const uint8_t ff_interleaved_ue_golomb_vlc_code[256];
extern const int8_t ff_interleaved_se_golomb_vlc_code[256];
extern const uint8_t ff_interleaved_dirac_golomb_vlc_code[256];
-/**
- * Read an unsigned Exp-Golomb code in the range 0 to 8190.
- *
- * @returns the read value or a negative error code.
- */
-static inline int get_ue_golomb(GetBitContext *gb)
+static inline int get_ue_golomb_internal(GetBitContext *gb, int emit_error_msg)
{
unsigned int buf;
@@ -67,7 +62,8 @@ static inline int get_ue_golomb(GetBitContext *gb)
} else {
int log = 2 * av_log2(buf) - 31;
if (log < 0) {
-av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
+if (emit_error_msg)
+av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
return AVERROR_INVALIDDATA;
}
buf >>= log;
@@ -92,7 +88,8 @@ static inline int get_ue_golomb(GetBitContext *gb)
LAST_SKIP_BITS(re, gb, 32 - log);
CLOSE_READER(re, gb);
if (log < 7) {
-av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
+if (emit_error_msg)
+av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
return AVERROR_INVALIDDATA;
}
buf >>= log;
@@ -103,6 +100,25 @@ static inline int get_ue_golomb(GetBitContext *gb)
#endif
}
+/**
+ * Read an unsigned Exp-Golomb code in the range 0 to 8190.
+ *
+ * @returns the read value or a negative error code.
+ */
+static inline int get_ue_golomb(GetBitContext *gb)
+{
+return get_ue_golomb_internal(gb, 1);
+}
+
+/**
+ * Variant of get_ue_golomb that does not emit an error message
+ * if the number is outside the permissible range.
+ */
+static inline int get_ue_golomb2(GetBitContext *gb)
+{
+return get_ue_golomb_internal(gb, 0);
+}
+
/**
* Read an unsigned Exp-Golomb code in the range 0 to UINT32_MAX-1.
*/
--
2.20.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
