[FFmpeg-devel] [PATCH] lavf/flvenc: apply various log fixes/clarifications

[Clément Bœsch]

On Thu, Sep 06, 2012 at 03:11:40PM +0200, Stefano Sabatini wrote:
> Should improve user feedback in case of errors.
> ---
>  libavformat/flvenc.c |   27 ++++++++++++++++-----------
>  1 files changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/libavformat/flvenc.c b/libavformat/flvenc.c
> index 69f1eac..3cf2b1b 100644
> --- a/libavformat/flvenc.c
> +++ b/libavformat/flvenc.c
> @@ -80,11 +80,11 @@ static int get_audio_flags(AVFormatContext *s, AVCodecContext *enc)
>      else if (enc->codec_id == AV_CODEC_ID_SPEEX) {
>          if (enc->sample_rate != 16000) {
>              av_log(s, AV_LOG_ERROR,
> -                   "flv only supports wideband (16kHz) Speex audio\n");
> +                   "FLV only supports wideband (16kHz) Speex audio\n");
>              return AVERROR(EINVAL);
>          }
>          if (enc->channels != 1) {
> -            av_log(s, AV_LOG_ERROR, "flv only supports mono Speex audio\n");
> +            av_log(s, AV_LOG_ERROR, "FLV only supports mono Speex audio\n");
>              return AVERROR(EINVAL);
>          }
>          return FLV_CODECID_SPEEX | FLV_SAMPLERATE_11025HZ | FLV_SAMPLESSIZE_16BIT;
> @@ -108,8 +108,8 @@ static int get_audio_flags(AVFormatContext *s, AVCodecContext *enc)
>              }
>          default:
>              av_log(s, AV_LOG_ERROR,
> -                   "flv does not support that sample rate, "
> -                   "choose from (44100, 22050, 11025).\n");
> +                   "FLV does not support sample rate %d, "
> +                   "choose from (44100, 22050, 11025)\n", enc->sample_rate);
>              return AVERROR(EINVAL);
>          }
>      }
> @@ -151,7 +151,8 @@ static int get_audio_flags(AVFormatContext *s, AVCodecContext *enc)
>          flags |= enc->codec_tag << 4;
>          break;
>      default:
> -        av_log(s, AV_LOG_ERROR, "codec not compatible with flv\n");
> +        av_log(s, AV_LOG_ERROR, "Audio codec '%s' not compatible with FLV\n",
> +               avcodec_get_name(enc->codec_id));
>          return AVERROR(EINVAL);
>      }
>  
> @@ -213,7 +214,8 @@ static int flv_write_header(AVFormatContext *s)
>              }
>              video_enc = enc;
>              if (enc->codec_tag == 0) {
> -                av_log(s, AV_LOG_ERROR, "video codec not compatible with flv\n");
> +                av_log(s, AV_LOG_ERROR, "Video codec '%s' for stream %d is not compatible with FLV\n",
> +                       avcodec_get_name(enc->codec_id), i);
>                  return AVERROR(EINVAL);
>              }
>              break;
> @@ -224,13 +226,15 @@ static int flv_write_header(AVFormatContext *s)
>              break;
>          case AVMEDIA_TYPE_DATA:
>              if (enc->codec_id != AV_CODEC_ID_TEXT) {
> -                av_log(s, AV_LOG_ERROR, "codec not compatible with flv\n");
> +                av_log(s, AV_LOG_ERROR, "Data codec '%s' for stream %d is not compatible with FLV\n",
> +                       avcodec_get_name(enc->codec_id), i);
>                  return AVERROR_INVALIDDATA;
>              }
>              data_enc = enc;
>              break;
>          default:
> -            av_log(s, AV_LOG_ERROR, "codec not compatible with flv\n");
> +            av_log(s, AV_LOG_ERROR, "Codec type '%s' for stream %d is not compatible with FLV\n",
> +                   av_get_media_type_string(enc->codec_type), i);
>              return AVERROR(EINVAL);
>          }
>          avpriv_set_pts_info(s->streams[i], 32, 1, 1000); /* 32 bit pts in ms */
> @@ -343,7 +347,7 @@ static int flv_write_header(AVFormatContext *s)
>              ||!strcmp(tag->key, "duration")
>              ||!strcmp(tag->key, "onMetaData")
>          ){
> -            av_log(s, AV_LOG_DEBUG, "ignoring metadata for %s\n", tag->key);
> +            av_log(s, AV_LOG_DEBUG, "Ignoring metadata for %s\n", tag->key);
>              continue;
>          }
>          put_amf_string(pb, tag->key);
> @@ -460,7 +464,7 @@ static int flv_write_packet(AVFormatContext *s, AVPacket *pkt)
>          flags = enc->codec_tag;
>          if (flags == 0) {
>              av_log(s, AV_LOG_ERROR,
> -                   "video codec %s not compatible with flv\n",
> +                   "Video codec '%s' is not compatible with FLV\n",
>                     avcodec_get_name(enc->codec_id));
>              return AVERROR(EINVAL);
>          }

This part LGTM

> @@ -488,7 +492,8 @@ static int flv_write_packet(AVFormatContext *s, AVPacket *pkt)
>                  return ret;
>      } else if (enc->codec_id == AV_CODEC_ID_AAC && pkt->size > 2 &&
>                 (AV_RB16(pkt->data) & 0xfff0) == 0xfff0) {
> -        av_log(s, AV_LOG_ERROR, "malformated aac bitstream, use -absf aac_adtstoasc\n");
> +        av_log(s, AV_LOG_ERROR, "Malformated AAC bitstream detected: "
> +               "use audio bistream filter 'aac_adtstoasc' to fix it\n");
>          return AVERROR_INVALIDDATA;
>      }

I think it's better to provide the copy-paste-me option so the user
doesn't wonder how to set that option.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120906/3915667e/attachment.asc>