From: Marth64 <marth64@proxyid.net>
To: ffmpeg-devel@ffmpeg.org
Cc: Marth64 <marth64@proxyid.net>
Subject: [FFmpeg-devel] [PATCH] avformat/dvdvideodec: remove `if ((ret = ...) < 0)` pattern
Date: Tue, 26 Mar 2024 15:17:49 -0500
Message-ID: <20240326201749.1543553-1-marth64@proxyid.net> (raw)
Recent advice plus my own experience agree that this pattern
is error-prone. Instead, set `ret` in its own line and do
the error validation after. Also, explicitly return 0 on success
in dvdvideo_chapters_setup_preindex()
Signed-off-by: Marth64 <marth64@proxyid.net>
---
libavformat/dvdvideodec.c | 132 +++++++++++++++++++++++++-------------
1 file changed, 86 insertions(+), 46 deletions(-)
diff --git a/libavformat/dvdvideodec.c b/libavformat/dvdvideodec.c
index c94e7f7fe6..000f9c5c9b 100644
--- a/libavformat/dvdvideodec.c
+++ b/libavformat/dvdvideodec.c
@@ -900,7 +900,7 @@ static int dvdvideo_chapters_setup_preindex(AVFormatContext *s)
{
DVDVideoDemuxContext *c = s->priv_data;
- int ret = 0, interrupt = 0;
+ int ret, interrupt = 0;
int nb_chapters = 0, last_ptt = c->opt_chapter_start;
uint64_t cur_chapter_offset = 0, cur_chapter_duration = 0;
DVDVideoPlaybackState state = {0};
@@ -909,9 +909,10 @@ static int dvdvideo_chapters_setup_preindex(AVFormatContext *s)
int nav_event;
if (c->opt_chapter_start == c->opt_chapter_end)
- return ret;
+ return 0;
- if ((ret = dvdvideo_play_open(s, &state)) < 0)
+ ret = dvdvideo_play_open(s, &state);
+ if (ret < 0)
return ret;
if (state.pgc->nr_of_programs == 1)
@@ -1058,7 +1059,7 @@ static int dvdvideo_video_stream_setup(AVFormatContext *s)
{
DVDVideoDemuxContext *c = s->priv_data;
- int ret = 0;
+ int ret;
DVDVideoVTSVideoStreamEntry entry = {0};
video_attr_t video_attr;
@@ -1068,14 +1069,11 @@ static int dvdvideo_video_stream_setup(AVFormatContext *s)
else
video_attr = c->vts_ifo->vtsi_mat->vts_video_attr;
- if ((ret = dvdvideo_video_stream_analyze(s, video_attr, &entry)) < 0 ||
- (ret = dvdvideo_video_stream_add(s, &entry, AVSTREAM_PARSE_HEADERS)) < 0) {
-
- av_log(s, AV_LOG_ERROR, "Unable to add video stream\n");
+ ret = dvdvideo_video_stream_analyze(s, video_attr, &entry);
+ if (ret < 0)
return ret;
- }
- return 0;
+ return dvdvideo_video_stream_add(s, &entry, AVSTREAM_PARSE_HEADERS);
}
static int dvdvideo_audio_stream_analyze(AVFormatContext *s, audio_attr_t audio_attr,
@@ -1219,7 +1217,7 @@ static int dvdvideo_audio_stream_add_all(AVFormatContext *s)
{
DVDVideoDemuxContext *c = s->priv_data;
- int ret = 0;
+ int ret;
int nb_streams;
if (c->opt_menu)
@@ -1241,8 +1239,9 @@ static int dvdvideo_audio_stream_add_all(AVFormatContext *s)
if (!(c->play_state.pgc->audio_control[i] & 0x8000))
continue;
- if ((ret = dvdvideo_audio_stream_analyze(s, audio_attr, c->play_state.pgc->audio_control[i],
- &entry)) < 0)
+ ret = dvdvideo_audio_stream_analyze(s, audio_attr, c->play_state.pgc->audio_control[i],
+ &entry);
+ if (ret < 0)
goto break_error;
/* IFO structures can declare duplicate entries for the same startcode */
@@ -1250,7 +1249,8 @@ static int dvdvideo_audio_stream_add_all(AVFormatContext *s)
if (s->streams[j]->id == entry.startcode)
continue;
- if ((ret = dvdvideo_audio_stream_add(s, &entry, AVSTREAM_PARSE_HEADERS)) < 0)
+ ret = dvdvideo_audio_stream_add(s, &entry, AVSTREAM_PARSE_HEADERS);
+ if (ret < 0)
goto break_error;
continue;
@@ -1302,7 +1302,8 @@ static int dvdvideo_subp_stream_add(AVFormatContext *s, DVDVideoPGCSubtitleStrea
st->codecpar->codec_type = AVMEDIA_TYPE_SUBTITLE;
st->codecpar->codec_id = AV_CODEC_ID_DVD_SUBTITLE;
- if ((ret = ff_dvdclut_palette_extradata_cat(entry->clut, FF_DVDCLUT_CLUT_SIZE, st->codecpar)) < 0)
+ ret = ff_dvdclut_palette_extradata_cat(entry->clut, FF_DVDCLUT_CLUT_SIZE, st->codecpar);
+ if (ret < 0)
return ret;
if (entry->lang_iso)
@@ -1326,12 +1327,13 @@ static int dvdvideo_subp_stream_add_internal(AVFormatContext *s, uint32_t offset
subp_attr_t subp_attr,
enum DVDVideoSubpictureViewport viewport)
{
- int ret = 0;
+ int ret;
DVDVideoPGCSubtitleStreamEntry entry = {0};
entry.viewport = viewport;
- if ((ret = dvdvideo_subp_stream_analyze(s, offset, subp_attr, &entry)) < 0)
+ ret = dvdvideo_subp_stream_analyze(s, offset, subp_attr, &entry);
+ if (ret < 0)
goto end_error;
/* IFO structures can declare duplicate entries for the same startcode */
@@ -1339,7 +1341,8 @@ static int dvdvideo_subp_stream_add_internal(AVFormatContext *s, uint32_t offset
if (s->streams[i]->id == entry.startcode)
return 0;
- if ((ret = dvdvideo_subp_stream_add(s, &entry, AVSTREAM_PARSE_HEADERS)) < 0)
+ ret = dvdvideo_subp_stream_add(s, &entry, AVSTREAM_PARSE_HEADERS);
+ if (ret < 0)
goto end_error;
return 0;
@@ -1363,7 +1366,7 @@ static int dvdvideo_subp_stream_add_all(AVFormatContext *s)
for (int i = 0; i < nb_streams; i++) {
- int ret = 0;
+ int ret;
uint32_t subp_control;
subp_attr_t subp_attr;
video_attr_t video_attr;
@@ -1387,29 +1390,35 @@ static int dvdvideo_subp_stream_add_all(AVFormatContext *s)
/* 4:3 */
if (!video_attr.display_aspect_ratio) {
- if ((ret = dvdvideo_subp_stream_add_internal(s, subp_control >> 24, subp_attr,
- DVDVIDEO_SUBP_VIEWPORT_FULLSCREEN)) < 0)
+ ret = dvdvideo_subp_stream_add_internal(s, subp_control >> 24, subp_attr,
+ DVDVIDEO_SUBP_VIEWPORT_FULLSCREEN);
+ if (ret < 0)
return ret;
continue;
}
/* 16:9 */
- if (( ret = dvdvideo_subp_stream_add_internal(s, subp_control >> 16, subp_attr,
- DVDVIDEO_SUBP_VIEWPORT_WIDESCREEN)) < 0)
+ ret = dvdvideo_subp_stream_add_internal(s, subp_control >> 16, subp_attr,
+ DVDVIDEO_SUBP_VIEWPORT_WIDESCREEN);
+ if (ret < 0)
return ret;
/* 16:9 letterbox */
- if (video_attr.permitted_df == 2 || video_attr.permitted_df == 0)
- if ((ret = dvdvideo_subp_stream_add_internal(s, subp_control >> 8, subp_attr,
- DVDVIDEO_SUBP_VIEWPORT_LETTERBOX)) < 0)
+ if (video_attr.permitted_df == 2 || video_attr.permitted_df == 0) {
+ ret = dvdvideo_subp_stream_add_internal(s, subp_control >> 8, subp_attr,
+ DVDVIDEO_SUBP_VIEWPORT_LETTERBOX);
+ if (ret < 0)
return ret;
+ }
/* 16:9 pan-and-scan */
- if (video_attr.permitted_df == 1 || video_attr.permitted_df == 0)
- if ((ret = dvdvideo_subp_stream_add_internal(s, subp_control, subp_attr,
- DVDVIDEO_SUBP_VIEWPORT_PANSCAN)) < 0)
+ if (video_attr.permitted_df == 1 || video_attr.permitted_df == 0) {
+ ret = dvdvideo_subp_stream_add_internal(s, subp_control, subp_attr,
+ DVDVIDEO_SUBP_VIEWPORT_PANSCAN);
+ if (ret < 0)
return ret;
+ }
}
return 0;
@@ -1433,7 +1442,7 @@ static int dvdvideo_subdemux_read_data(void *opaque, uint8_t *buf, int buf_size)
AVFormatContext *s = opaque;
DVDVideoDemuxContext *c = s->priv_data;
- int ret = 0;
+ int ret;
int nav_event;
if (c->play_end)
@@ -1471,7 +1480,7 @@ static int dvdvideo_subdemux_open(AVFormatContext *s)
{
DVDVideoDemuxContext *c = s->priv_data;
extern const FFInputFormat ff_mpegps_demuxer;
- int ret = 0;
+ int ret;
if (!(c->mpeg_buf = av_mallocz(DVDVIDEO_BLOCK_SIZE)))
return AVERROR(ENOMEM);
@@ -1483,7 +1492,8 @@ static int dvdvideo_subdemux_open(AVFormatContext *s)
if (!(c->mpeg_ctx = avformat_alloc_context()))
return AVERROR(ENOMEM);
- if ((ret = ff_copy_whiteblacklists(c->mpeg_ctx, s)) < 0) {
+ ret = ff_copy_whiteblacklists(c->mpeg_ctx, s);
+ if (ret < 0) {
avformat_free_context(c->mpeg_ctx);
c->mpeg_ctx = NULL;
@@ -1506,7 +1516,7 @@ static int dvdvideo_read_header(AVFormatContext *s)
{
DVDVideoDemuxContext *c = s->priv_data;
- int ret = 0;
+ int ret;
if (c->opt_menu) {
if (c->opt_region ||
@@ -1539,12 +1549,25 @@ static int dvdvideo_read_header(AVFormatContext *s)
c->opt_pg = 1;
}
- if ((ret = dvdvideo_ifo_open(s)) < 0 ||
- (ret = dvdvideo_menu_open(s, &c->play_state)) < 0 ||
- (ret = dvdvideo_subdemux_open(s)) < 0 ||
- (ret = dvdvideo_video_stream_setup(s)) < 0 ||
- (ret = dvdvideo_audio_stream_add_all(s)) < 0)
- return ret;
+ ret = dvdvideo_ifo_open(s);
+ if (ret < 0)
+ return ret;
+
+ ret = dvdvideo_menu_open(s, &c->play_state);
+ if (ret < 0)
+ return ret;
+
+ ret = dvdvideo_subdemux_open(s);
+ if (ret < 0)
+ return ret;
+
+ ret = dvdvideo_video_stream_setup(s);
+ if (ret < 0)
+ return ret;
+
+ ret = dvdvideo_audio_stream_add_all(s);
+ if (ret < 0)
+ return ret;
return 0;
}
@@ -1568,17 +1591,34 @@ static int dvdvideo_read_header(AVFormatContext *s)
}
}
- if ((ret = dvdvideo_ifo_open(s)) < 0)
+ ret = dvdvideo_ifo_open(s);
+ if (ret < 0)
return ret;
- if (!c->opt_pgc && c->opt_preindex && (ret = dvdvideo_chapters_setup_preindex(s)) < 0)
+ if (!c->opt_pgc && c->opt_preindex) {
+ ret = dvdvideo_chapters_setup_preindex(s);
+ if (ret < 0)
+ return ret;
+ }
+
+ ret = dvdvideo_play_open(s, &c->play_state);
+ if (ret < 0)
return ret;
- if ((ret = dvdvideo_play_open(s, &c->play_state)) < 0 ||
- (ret = dvdvideo_subdemux_open(s)) < 0 ||
- (ret = dvdvideo_video_stream_setup(s)) < 0 ||
- (ret = dvdvideo_audio_stream_add_all(s)) < 0 ||
- (ret = dvdvideo_subp_stream_add_all(s)) < 0)
+ ret = dvdvideo_subdemux_open(s);
+ if (ret < 0)
+ return ret;
+
+ ret = dvdvideo_video_stream_setup(s);
+ if (ret < 0)
+ return ret;
+
+ ret = dvdvideo_audio_stream_add_all(s);
+ if (ret < 0)
+ return ret;
+
+ ret = dvdvideo_subp_stream_add_all(s);
+ if (ret < 0)
return ret;
if (!c->opt_pgc && !c->opt_preindex)
--
2.34.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-request@ffmpeg.org with subject "unsubscribe".
next reply other threads:[~2024-03-26 20:18 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-26 20:17 Marth64 [this message]
2024-03-27 11:27 ` Stefano Sabatini
2024-03-27 12:21 ` Anton Khirnov
2024-03-28 15:52 ` Marth64
2024-03-27 11:32 ` Andreas Rheinhardt
2024-03-27 12:08 ` Stefano Sabatini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240326201749.1543553-1-marth64@proxyid.net \
--to=marth64@proxyid.net \
--cc=ffmpeg-devel@ffmpeg.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git