Re: [FFmpeg-devel] [PATCH 5/7] avformat/segafilmenc: Remove redundant checks
On Fri, Feb 21, 2020 at 02:10:00PM +0100, Paul B Mahol wrote: > lgtm will apply thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The greatest way to live with honor in this world is to be what we pretend to be. -- Socrates 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 5/7] avformat/segafilmenc: Remove redundant checks
lgtm
On 1/14/20, Andreas Rheinhardt wrote:
> If an audio stream is present, the Sega FILM muxer checks for its
> compability with the container during init, so that the very same check
> needn't be repeated during writing the trailer.
>
> Essentially the same is true for the presence of a video stream: It has
> already been checked during init. Furthermore, after the check for the
> presence of a video stream succeeded, a pointer is set to point to the
> video stream. Yet said pointer (which was NULL before) will be
> derefenced anyway regardless of the result of the check. Coverity thus
> complained about this in CID 1434155 and removing this pointless check
> will also fix this issue.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> libavformat/segafilmenc.c | 8 ++--
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/libavformat/segafilmenc.c b/libavformat/segafilmenc.c
> index 5ac60ea5c3..4f881f4f2f 100644
> --- a/libavformat/segafilmenc.c
> +++ b/libavformat/segafilmenc.c
> @@ -292,15 +292,9 @@ static int film_write_header(AVFormatContext
> *format_context)
>
> if (film->audio_index > -1)
> audio = format_context->streams[film->audio_index];
> -if (film->video_index > -1)
> -video = format_context->streams[film->video_index];
>
> if (audio != NULL) {
> audio_codec = get_audio_codec_id(audio->codecpar->codec_id);
> -if (audio_codec < 0) {
> -av_log(format_context, AV_LOG_ERROR, "Incompatible audio stream
> format.\n");
> -return AVERROR(EINVAL);
> -}
> }
>
> /* First, write the FILM header; this is very simple */
> @@ -317,6 +311,8 @@ static int film_write_header(AVFormatContext
> *format_context)
> ffio_wfourcc(pb, "FDSC");
> avio_wb32(pb, 0x20); /* Size of FDSC chunk */
>
> +video = format_context->streams[film->video_index];
> +
> /* The only two supported codecs; raw video is rare */
> switch (video->codecpar->codec_id) {
> case AV_CODEC_ID_CINEPAK:
> --
> 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".
___
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 5/7] avformat/segafilmenc: Remove redundant checks
Andreas Rheinhardt:
> Andreas Rheinhardt:
>> If an audio stream is present, the Sega FILM muxer checks for its
>> compability with the container during init, so that the very same check
>> needn't be repeated during writing the trailer.
>>
>> Essentially the same is true for the presence of a video stream: It has
>> already been checked during init. Furthermore, after the check for the
>> presence of a video stream succeeded, a pointer is set to point to the
>> video stream. Yet said pointer (which was NULL before) will be
>> derefenced anyway regardless of the result of the check. Coverity thus
>> complained about this in CID 1434155 and removing this pointless check
>> will also fix this issue.
>>
>> Signed-off-by: Andreas Rheinhardt
>> ---
>> libavformat/segafilmenc.c | 8 ++--
>> 1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/libavformat/segafilmenc.c b/libavformat/segafilmenc.c
>> index 5ac60ea5c3..4f881f4f2f 100644
>> --- a/libavformat/segafilmenc.c
>> +++ b/libavformat/segafilmenc.c
>> @@ -292,15 +292,9 @@ static int film_write_header(AVFormatContext
>> *format_context)
>>
>> if (film->audio_index > -1)
>> audio = format_context->streams[film->audio_index];
>> -if (film->video_index > -1)
>> -video = format_context->streams[film->video_index];
>>
>> if (audio != NULL) {
>> audio_codec = get_audio_codec_id(audio->codecpar->codec_id);
>> -if (audio_codec < 0) {
>> -av_log(format_context, AV_LOG_ERROR, "Incompatible audio stream
>> format.\n");
>> -return AVERROR(EINVAL);
>> -}
>> }
>>
>> /* First, write the FILM header; this is very simple */
>> @@ -317,6 +311,8 @@ static int film_write_header(AVFormatContext
>> *format_context)
>> ffio_wfourcc(pb, "FDSC");
>> avio_wb32(pb, 0x20); /* Size of FDSC chunk */
>>
>> +video = format_context->streams[film->video_index];
>> +
>> /* The only two supported codecs; raw video is rare */
>> switch (video->codecpar->codec_id) {
>> case AV_CODEC_ID_CINEPAK:
>>
>
> Ping.
>
> - Andreas
>
Another ping.
- 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 5/7] avformat/segafilmenc: Remove redundant checks
Andreas Rheinhardt:
> If an audio stream is present, the Sega FILM muxer checks for its
> compability with the container during init, so that the very same check
> needn't be repeated during writing the trailer.
>
> Essentially the same is true for the presence of a video stream: It has
> already been checked during init. Furthermore, after the check for the
> presence of a video stream succeeded, a pointer is set to point to the
> video stream. Yet said pointer (which was NULL before) will be
> derefenced anyway regardless of the result of the check. Coverity thus
> complained about this in CID 1434155 and removing this pointless check
> will also fix this issue.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> libavformat/segafilmenc.c | 8 ++--
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/libavformat/segafilmenc.c b/libavformat/segafilmenc.c
> index 5ac60ea5c3..4f881f4f2f 100644
> --- a/libavformat/segafilmenc.c
> +++ b/libavformat/segafilmenc.c
> @@ -292,15 +292,9 @@ static int film_write_header(AVFormatContext
> *format_context)
>
> if (film->audio_index > -1)
> audio = format_context->streams[film->audio_index];
> -if (film->video_index > -1)
> -video = format_context->streams[film->video_index];
>
> if (audio != NULL) {
> audio_codec = get_audio_codec_id(audio->codecpar->codec_id);
> -if (audio_codec < 0) {
> -av_log(format_context, AV_LOG_ERROR, "Incompatible audio stream
> format.\n");
> -return AVERROR(EINVAL);
> -}
> }
>
> /* First, write the FILM header; this is very simple */
> @@ -317,6 +311,8 @@ static int film_write_header(AVFormatContext
> *format_context)
> ffio_wfourcc(pb, "FDSC");
> avio_wb32(pb, 0x20); /* Size of FDSC chunk */
>
> +video = format_context->streams[film->video_index];
> +
> /* The only two supported codecs; raw video is rare */
> switch (video->codecpar->codec_id) {
> case AV_CODEC_ID_CINEPAK:
>
Ping.
- 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".
[FFmpeg-devel] [PATCH 5/7] avformat/segafilmenc: Remove redundant checks
If an audio stream is present, the Sega FILM muxer checks for its
compability with the container during init, so that the very same check
needn't be repeated during writing the trailer.
Essentially the same is true for the presence of a video stream: It has
already been checked during init. Furthermore, after the check for the
presence of a video stream succeeded, a pointer is set to point to the
video stream. Yet said pointer (which was NULL before) will be
derefenced anyway regardless of the result of the check. Coverity thus
complained about this in CID 1434155 and removing this pointless check
will also fix this issue.
Signed-off-by: Andreas Rheinhardt
---
libavformat/segafilmenc.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/libavformat/segafilmenc.c b/libavformat/segafilmenc.c
index 5ac60ea5c3..4f881f4f2f 100644
--- a/libavformat/segafilmenc.c
+++ b/libavformat/segafilmenc.c
@@ -292,15 +292,9 @@ static int film_write_header(AVFormatContext
*format_context)
if (film->audio_index > -1)
audio = format_context->streams[film->audio_index];
-if (film->video_index > -1)
-video = format_context->streams[film->video_index];
if (audio != NULL) {
audio_codec = get_audio_codec_id(audio->codecpar->codec_id);
-if (audio_codec < 0) {
-av_log(format_context, AV_LOG_ERROR, "Incompatible audio stream
format.\n");
-return AVERROR(EINVAL);
-}
}
/* First, write the FILM header; this is very simple */
@@ -317,6 +311,8 @@ static int film_write_header(AVFormatContext
*format_context)
ffio_wfourcc(pb, "FDSC");
avio_wb32(pb, 0x20); /* Size of FDSC chunk */
+video = format_context->streams[film->video_index];
+
/* The only two supported codecs; raw video is rare */
switch (video->codecpar->codec_id) {
case AV_CODEC_ID_CINEPAK:
--
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".
