Skip to content

Return code of av_write_trailer should be checked for negative, not 0. #241

Description

@jean343

🐞 Bug Report: Improper Return Code Check for av_write_trailer

Summary

The return value of av_write_trailer() is incorrectly checked for a return value of 0, instead of for any negative value which indicates an error. This can result in silently ignoring legitimate errors.

Problem Details

1. FFmpeg Code Snippet

if ((ret = av_write_trailer(ofmt_ctx)) < 0)
    fprintf(stderr, "Failed to write trailer %s\n", av_err2str(ret));

Correct: The FFmpeg code checks for negative return values (ret < 0), which is the correct way to detect errors.


2. Rust Wrapper Code Snippet

pub fn write_trailer(&mut self) -> Result<(), Error> {
    unsafe {
        match av_write_trailer(self.as_mut_ptr()) {
            0 => Ok(()),
            e => Err(Error::from(e)),
        }
    }
}

Incorrect: This match statement assumes only 0 is valid, and all other values are errors.
🛑 However, FFmpeg functions return negative integers on error, and non-zero positive return values may be valid, depending on context. This implementation would incorrectly treat any non-zero positive return as an error.


Suggested Fix

Rust Wrapper: Properly check for < 0 instead of != 0

Replace with:

pub fn write_trailer(&mut self) -> Result<(), Error> {
    unsafe {
        let ret = av_write_trailer(self.as_mut_ptr());
        if ret < 0 {
            Err(Error::from(ret))
        } else {
            Ok(())
        }
    }
}

This ensures only true errors (i.e., negative return values) are treated as failures, aligning with FFmpeg's conventions.

Impact

  • Misleading Error returned on valid positive return codes.
  • Possible masking of specific success or warning codes from FFmpeg.
  • Inconsistent behavior between FFmpeg and wrapper.

Reproduction:

        let input_path = "./1747611943353_15147.ts";
        let mut ictx = input(&input_path)?;

        let video_index = ictx
            .streams()
            .best(ffmpeg_next::media::Type::Video)
            .ok_or(ffmpeg_next::Error::StreamNotFound)?
            .index();

        let mut segment_index = 0;
        let mut segment_ctx: Option<Output> = None;
        let mut out_stream_index = 0;

        let in_stream = ictx.stream(video_index).unwrap();
        let parameters = in_stream.parameters().clone();
        let input_time_base = in_stream.time_base();

        for (stream, mut packet) in ictx.packets() {
            if stream.index() != video_index {
                continue;
            }

            // On keyframe or no open output, start a new segment
            let is_key = packet.is_key();
            if is_key || segment_ctx.is_none() {
                if let Some(mut ctx) = segment_ctx.take() {
                    ctx.write_trailer()?;
                }

                let filename = format!("./segment_{:03}.mp4", segment_index);
                segment_index += 1;

                let mut octx = format::output(&filename)?;
                let mut opts = Dictionary::new();
                opts.set("movflags", "frag_keyframe+empty_moov+default_base_moof");

                // Add stream and copy parameters
                let mut out_stream = octx.add_stream(codec::encoder::find(codec::Id::None))?;
                out_stream.set_parameters(parameters.clone());
                out_stream_index = out_stream.index();

                octx.write_header_with(opts)?;
                segment_ctx = Some(octx);
            }

            // Write packet into segment
            if let Some(ref mut ctx) = segment_ctx {
                packet.rescale_ts(
                    input_time_base,
                    ctx.stream(out_stream_index).unwrap().time_base(),
                );
                packet.set_position(-1);
                packet.write_interleaved(ctx)?;
            }
        }

        // Finalize last segment
        if let Some(mut ctx) = segment_ctx {
            ctx.write_trailer()?;
        }

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions