raspberrypi / rpicam-apps

BSD 2-Clause "Simplified" License
420 stars 228 forks source link

New LibAV segment ... implementation (PR 537) doesn't work #580

Open oliversturm opened 1 year ago

oliversturm commented 1 year ago

I'm trying to use the --segment/libav feature in this PR, but it doesn't seem to do anything. I built from the repo based on these instructions, cloned today from the main branch.

My complete command is as follows -- including all the options in case something makes a difference.

/usr/local/bin/libcamera-vid -n --awb auto -t 0 --exposure normal --autofocus-mode auto --tuning-file /usr/local/share/libcamera/ipa/rpi/vc4/imx708.json --width 1920 --height 1080 --codec libav --bitrate 10000000 --vflip --hflip --segment 20000 -v 2 --libav-audio --audio-bitrate 16384 --audio-device alsa_input.usb-C-Media_Electronics_Inc._USB_PnP_Sound_Device-00.mono-fallback -o "/mnt/video/vid-%05d.mkv"

I expected this to create a new numbered file every 20 seconds, but instead I get just one file that is literally called vid-%05d.mkv.

Any idea what I'm doing wrong?

oliversturm commented 1 year ago

Reading some source code, is it possible that this line should not say microseconds towards the end?

timestamp_us / 1000 - file_start_time_ms_ > options_->segment.get<std::chrono::microseconds>()) ||

The --segment option is described elsewhere as accepting milliseconds, so this sticks out -- not sure what consequences it could have.

oliversturm commented 1 year ago

One more thing that looks odd to me (but keep in mind my C++ might be a bit rusty these days). In output/file_output.cpp there this line that formats the output filename:

int n = snprintf(filename, sizeof(filename), options_->output.c_str(), count_);

However, this method in output/output.cpp seems to instantiate the type Output as long as I use --codec libav -- FileOutput is not used in that case, but Output does not have the logic to format the filename.

Output *Output::Create(VideoOptions const *options)
{
        if (options->codec == "libav")
                return new Output(options);

        if (strncmp(options->output.c_str(), "udp://", 6) == 0 || strncmp(options->output.c_str(), "tcp://", 6) == 0)
                return new NetOutput(options);
        else if (options->circular)
                return new CircularOutput(options);
        else if (!options->output.empty())
                return new FileOutput(options);
        else
                return new Output(options);
}

Okay, I'll shut up now. I assume that somebody tested the functionality when the PR was merged, and if it worked then I'm on the wrong track looking for a general bug while I'm probably just passing the options wrong.

naushir commented 1 year ago

Unfortunately we have also noticed a few issues with the libav segmenting changes. In fact I had to revert these changes in commit 15e9007db75b832bd7473c796ab62265ebb18918 temporarily until we get some time to revisit and fix up these issue.

oliversturm commented 1 year ago

I see, thanks for letting me know. I didn't double-check current code against the merged PR so I didn't notice...

naushir commented 1 year ago

Yeah, sorry about the confusion. Once we get some resources free, we will work on getting this functionality back in and working correctly.

koster-mobile commented 1 year ago

I had to checkout 9e17265 and segmenting with libav seems to work there. On the later commits it produces a single file without interpolation of the counter into the filename.

What I noticed is that implementation has an issue regarding produced .mp4 files - their timestamps are increasing per file. So for example the first file have ts 0 to 30 but the second has 30..60, third has 60..90 etc. It confuses video players like VLC and HTML video. I want to create a video recorder and only libav codec consumes little cpu and allows a variable bitrate. As a quick hack I added a code as a temporary workaround that erases ELST chunk out of mp4 container - and it works.

It is good news to hear that "correct" implementation is planned.

ddbaron commented 9 months ago

I was attempting to use --segment as part of a workaround for --circular bug https://github.com/raspberrypi/rpicam-apps/issues/651 when I experienced the subject bug and found this report; subscribed

koster-mobile commented 9 months ago

I was attempting to use --segment as part of a workaround for --circular bug https://github.com/raspberrypi/rpicam-apps/issues/651 when I experienced the subject bug and found this report; subscribed

I ended up using commit 07fc461 where segment functionality was not already removed for MP4 but it tooks patching the code to ensure stable split functionality with a keyframe at the start of the segment; using variable quality feature instead of just setting constant bitrate; boosting sound volume; some efforts were also put to make sound be recorded in sync without a gap increasing in time. But the hack with erasing ELST atom after MP4 segment is finished is still the key point of the solution :( I invested a lot of time trying to make a correct solution but every attempt either fails with lots of FFMPEG errors emitted or makes media desynchronization.

Not sure if something changed last months, don't want to spend time to see that it is still broken. I dont create a PR for the reason of the hack with ELST and there are also some side functions I put to the code for my particular project (making a JPG previews for each segment and another way of segment file naming).

Update: BTW I found a solution for segmenting just video without that ELST hack, it is setting out_fmt_ctx_->output_ts_offset value at the starting of the chunk. But audio track is being desynchronized. I need audio so I'm not using this approach.

xethm55 commented 2 months ago

@koster-mobile I am working on a project that would be beneficial to have segmentation while using libav. Would you be willing to post a patch? Thank You.