Fix usage of deprecated libavcodec functions #150
Conversation
|
@sea-bass This seems to work on Humble and Ubuntu 22.04 but Segfaults on Rolling on Ubuntu 24.04 when trying to use vp8, vp9 or h264 compression (mjpeg streaming works). Backtrace shows the seg fault occurs in |
|
I think I've got this finally working. Checked on Humble, Iron, Jazzy and Rolling. I can't get H264 to work (it never worked for me) but all of the other stream types (mjpeg, png, ros_compressed, vp8, vp9) are working. |
|
|
There was a problem hiding this comment.
Thanks for this contribution! You got to it before I was able to get my project's Jazzy upgrade underway :)
I will largely defer to @sea-bass here because I am not as well-versed in C++ as he is, but the changes seem sensical to me.
| RCLCPP_DEBUG_STREAM(nh_->get_logger(), "avcodec_send_frame() encoder flushed\n"); | ||
| } | ||
| #else | ||
| pkt.data = NULL; // packet data will be allocated by the encoder | ||
| pkt.size = 0; | ||
| if (avcodec_send_frame(codec_context_, frame_) < 0) | ||
| else if (ret == AVERROR(EAGAIN)) | ||
| { | ||
| RCLCPP_DEBUG_STREAM(nh_->get_logger(), "avcodec_send_frame() need output read out\n"); | ||
| } | ||
| if (ret < 0) | ||
| { | ||
| throw std::runtime_error("Error encoding video frame"); | ||
| } | ||
| if (avcodec_receive_packet(codec_context_, &pkt) < 0) | ||
|
|
||
| ret = avcodec_receive_packet(codec_context_, pkt); | ||
| bool got_packet = pkt->size > 0; | ||
| if (ret == AVERROR_EOF) | ||
| { | ||
| RCLCPP_DEBUG_STREAM(nh_->get_logger(), "avcodec_receive_packet() encoder flushed\n"); | ||
| } | ||
| else if (ret == AVERROR(EAGAIN)) | ||
| { | ||
| throw std::runtime_error("Error retrieving encoded packet"); | ||
| RCLCPP_DEBUG_STREAM(nh_->get_logger(), "avcodec_receive_packet() needs more input\n"); | ||
| got_packet = false; |
There was a problem hiding this comment.
Did you want to keep these debug logs before merging, or did you intend to remove them?
There was a problem hiding this comment.
The debug logs were backported from #103 . I don't see any reason to remove them.
| // Encode video at 1/0.95 to minimize delay | ||
| pkt.pts = (int64_t)(seconds / av_q2d(video_stream_->time_base) * 0.95); | ||
| if (pkt.pts <= 0) | ||
| pkt.pts = 1; | ||
| pkt.dts = AV_NOPTS_VALUE; | ||
| pkt->pts = (int64_t)(seconds / av_q2d(video_stream_->time_base) * 0.95); |
There was a problem hiding this comment.
This magic number of 0.95 would be nice to have as an actual constant defined in an anonymous namespace at the top of this file.
There was a problem hiding this comment.
I think this is out of scope for this PR. Instead of making it a constant I might add URL-controlled parameter for this (e.g. playback_speed) in the future.
Fixes web_video_server for Jazzy and Rolling distributions.
I also dropped backward compatibility with old ffmpeg libraries to make the code easier to maintain.