[FFmpeg-devel] avs2: Correct expression error and simplify log

Submitted by hwren on Aug. 6, 2018, 1:43 p.m.

Details

Message ID 1533562999-7502-1-git-send-email-hwrenx@126.com
State New
Headers show

Commit Message

hwren Aug. 6, 2018, 1:43 p.m.
Signed-off-by: hwren <hwrenx@126.com>
---
 doc/decoders.texi        | 2 +-
 doc/general.texi         | 2 +-
 libavcodec/avs2_parser.c | 2 +-
 libavcodec/codec_desc.c  | 2 +-
 libavcodec/libdavs2.c    | 5 ++---
 5 files changed, 6 insertions(+), 7 deletions(-)

Comments

Gyan Aug. 6, 2018, 2:21 p.m.
On 06-08-2018 07:13 PM, hwren wrote:

> -    .long_name      = NULL_IF_CONFIG_SMALL("Decoder for AVS2/IEEE 1857.4"),
> +    .long_name      = NULL_IF_CONFIG_SMALL("Decoder for AVS2-P2/IEEE1857.4"),

"Decoder for" is not needed. The flag indicates its role in `ffmpeg 
-codecs`.

Rest docs LGTM


Thanks,
Gyan
James Almer Aug. 6, 2018, 6:09 p.m.
On 8/6/2018 11:21 AM, Gyan Doshi wrote:
> 
> 
> On 06-08-2018 07:13 PM, hwren wrote:
> 
>> -    .long_name      = NULL_IF_CONFIG_SMALL("Decoder for AVS2/IEEE
>> 1857.4"),
>> +    .long_name      = NULL_IF_CONFIG_SMALL("Decoder for
>> AVS2-P2/IEEE1857.4"),
> 
> "Decoder for" is not needed. The flag indicates its role in `ffmpeg
> -codecs`.

The correct string considering this is a decoder using an external
library would be something like "libdavs2 AVS2-P2/IEEE1857.4"

> 
> Rest docs LGTM
> 
> 
> Thanks,
> Gyan
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
hwren Aug. 6, 2018, 11:34 p.m.
在 2018-08-07 02:09:34,"James Almer" <jamrial@gmail.com> 写道:
>On 8/6/2018 11:21 AM, Gyan Doshi wrote:
>> 
>> 
>> On 06-08-2018 07:13 PM, hwren wrote:
>> 
>>> -    .long_name      = NULL_IF_CONFIG_SMALL("Decoder for AVS2/IEEE
>>> 1857.4"),
>>> +    .long_name      = NULL_IF_CONFIG_SMALL("Decoder for
>>> AVS2-P2/IEEE1857.4"),
>> 
>> "Decoder for" is not needed. The flag indicates its role in `ffmpeg
>> -codecs`.
>
>The correct string considering this is a decoder using an external
>library would be something like "libdavs2 AVS2-P2/IEEE1857.4"

"AVS2-P2/IEEE1857.4 external decoder wrapper"?

>
>> 
>> Rest docs LGTM
>> 
>> 
>> Thanks,
>> Gyan
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>_______________________________________________
>ffmpeg-devel mailing list
>ffmpeg-devel@ffmpeg.org
>http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
James Almer Aug. 7, 2018, 3:07 a.m.
On 8/6/2018 8:34 PM, Huiwen Ren wrote:
> 
> 在 2018-08-07 02:09:34,"James Almer" <jamrial@gmail.com> 写道:
>> On 8/6/2018 11:21 AM, Gyan Doshi wrote:
>>>
>>>
>>> On 06-08-2018 07:13 PM, hwren wrote:
>>>
>>>> -    .long_name      = NULL_IF_CONFIG_SMALL("Decoder for AVS2/IEEE
>>>> 1857.4"),
>>>> +    .long_name      = NULL_IF_CONFIG_SMALL("Decoder for
>>>> AVS2-P2/IEEE1857.4"),
>>>
>>> "Decoder for" is not needed. The flag indicates its role in `ffmpeg
>>> -codecs`.
>>
>> The correct string considering this is a decoder using an external
>> library would be something like "libdavs2 AVS2-P2/IEEE1857.4"
> 
> "AVS2-P2/IEEE1857.4 external decoder wrapper"?

No, look at libopenh264dec.c, libvpxdec.c, etc. They all have a
long_name field listing the name of the external library followed by the
codec they support.

OpenH264 H.264 / AVC / MPEG-4 AVC / MPEG-4 part 10
libvpx VP9
libaom AV1
Fraunhofer FDK AAC
libopus Opus

So "libdavs2 AVS2-P2/IEEE1857.4" would be the correct string here.
Moritz Barsnick Aug. 7, 2018, 10:08 a.m.
On Mon, Aug 06, 2018 at 19:51:48 +0530, Gyan Doshi wrote:
> 
> 
> On 06-08-2018 07:13 PM, hwren wrote:
> 
> > -    .long_name      = NULL_IF_CONFIG_SMALL("Decoder for AVS2/IEEE 1857.4"),
> > +    .long_name      = NULL_IF_CONFIG_SMALL("Decoder for AVS2-P2/IEEE1857.4"),
> 
> "Decoder for" is not needed. The flag indicates its role in `ffmpeg 
> -codecs`.
> 
> Rest docs LGTM

And the change of codec description doesn't fit with the patch's commit
message. It should probably be split out into two patches, or the
commit message amended.

Moritz

Patch hide | download patch | download mbox

diff --git a/doc/decoders.texi b/doc/decoders.texi
index 9439b7b..31e96fb 100644
--- a/doc/decoders.texi
+++ b/doc/decoders.texi
@@ -49,7 +49,7 @@  top-field-first is assumed
 
 @section libdavs2
 
-AVS2/IEEE 1857.4 video decoder wrapper.
+AVS2-P2/IEEE1857.4 video decoder wrapper.
 
 This decoder allows libavcodec to decode AVS2 streams with davs2 library.
 
diff --git a/doc/general.texi b/doc/general.texi
index cd725bb..3d15840 100644
--- a/doc/general.texi
+++ b/doc/general.texi
@@ -19,7 +19,7 @@  explicitly requested by passing the appropriate flags to
 
 @section libdavs2
 
-FFmpeg can make use of the davs2 library for AVS2/IEEE 1857.4 video decoding.
+FFmpeg can make use of the davs2 library for AVS2-P2/IEEE1857.4 video decoding.
 
 Go to @url{https://github.com/pkuvcl/davs2} and follow the instructions for
 installing the library. Then pass @code{--enable-libdavs2} to configure to
diff --git a/libavcodec/avs2_parser.c b/libavcodec/avs2_parser.c
index 520e98a..1c9b342 100644
--- a/libavcodec/avs2_parser.c
+++ b/libavcodec/avs2_parser.c
@@ -1,5 +1,5 @@ 
 /*
- * AVS2/IEEE 1857.4 video parser.
+ * AVS2-P2/IEEE1857.4 video parser.
  * Copyright (c) 2018  Huiwen Ren <hwrenx@gmail.com>
  *
  * This file is part of FFmpeg.
diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
index 2fe4aaa..1cbb241 100644
--- a/libavcodec/codec_desc.c
+++ b/libavcodec/codec_desc.c
@@ -1398,7 +1398,7 @@  static const AVCodecDescriptor codec_descriptors[] = {
         .id        = AV_CODEC_ID_AVS2,
         .type      = AVMEDIA_TYPE_VIDEO,
         .name      = "avs2",
-        .long_name = NULL_IF_CONFIG_SMALL("AVS2/IEEE 1857.4"),
+        .long_name = NULL_IF_CONFIG_SMALL("AVS2-P2/IEEE1857.4"),
         .props     = AV_CODEC_PROP_LOSSY,
     },
     {
diff --git a/libavcodec/libdavs2.c b/libavcodec/libdavs2.c
index 12db1f9..80c2e2b 100644
--- a/libavcodec/libdavs2.c
+++ b/libavcodec/libdavs2.c
@@ -51,7 +51,7 @@  static av_cold int davs2_init(AVCodecContext *avctx)
 
     /* init the decoder */
     cad->param.threads      = avctx->thread_count;
-    cad->param.info_level   = 0;
+    cad->param.info_level   = DAVS2_LOG_WARNING;
     cad->decoder            = davs2_decoder_open(&cad->param);
 
     if (!cad->decoder) {
@@ -59,7 +59,6 @@  static av_cold int davs2_init(AVCodecContext *avctx)
         return AVERROR(EINVAL);
     }
 
-    av_log(avctx, AV_LOG_VERBOSE, "decoder created. %p\n", cad->decoder);
     return 0;
 }
 
@@ -163,7 +162,7 @@  static int davs2_decode_frame(AVCodecContext *avctx, void *data,
 
 AVCodec ff_libdavs2_decoder = {
     .name           = "libdavs2",
-    .long_name      = NULL_IF_CONFIG_SMALL("Decoder for AVS2/IEEE 1857.4"),
+    .long_name      = NULL_IF_CONFIG_SMALL("Decoder for AVS2-P2/IEEE1857.4"),
     .type           = AVMEDIA_TYPE_VIDEO,
     .id             = AV_CODEC_ID_AVS2,
     .priv_data_size = sizeof(DAVS2Context),