Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ffmpeg software decoder implementation #461

Open
wants to merge 12 commits into
base: beta-testing
Choose a base branch
from

Conversation

rrawther
Copy link
Collaborator

Implemented ffmpeg based software decoder
Modified videoDecode sample to allow ffmpeg based decoding

@kiritigowda kiritigowda added enhancement New feature or request ci:precheckin run mainline precheckin CI job labels Nov 26, 2024
Copy link
Contributor

@spolifroni-amd spolifroni-amd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the additional lines in the changelogs are for 6.3, please let me know.

CHANGELOG.md Show resolved Hide resolved
Copy link
Member

@AryanSalmanpour AryanSalmanpour left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a few minor formatting and some suggestions.

utils/ffmpegvideodecode/ffmpeg_video_dec.h Show resolved Hide resolved
utils/ffmpegvideodecode/ffmpeg_video_dec.h Show resolved Hide resolved
utils/ffmpegvideodecode/ffmpeg_video_dec.h Show resolved Hide resolved
utils/ffmpegvideodecode/ffmpeg_video_dec.h Show resolved Hide resolved
utils/ffmpegvideodecode/ffmpeg_video_dec.cpp Show resolved Hide resolved
utils/ffmpegvideodecode/ffmpeg_video_dec.cpp Show resolved Hide resolved
utils/ffmpegvideodecode/ffmpeg_video_dec.cpp Show resolved Hide resolved
utils/ffmpegvideodecode/ffmpeg_video_dec.cpp Show resolved Hide resolved
utils/ffmpegvideodecode/ffmpeg_video_dec.cpp Show resolved Hide resolved
utils/ffmpegvideodecode/ffmpeg_video_dec.cpp Show resolved Hide resolved
@@ -203,18 +214,26 @@ int main(int argc, char **argv) {
try {
std::size_t found_file = input_file_path.find_last_of('/');
std::cout << "info: Input file: " << input_file_path.substr(found_file + 1) << std::endl;
RocVideoDecoder *viddec;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we initialize to nullptr?

if (av_packets_.empty()) {
for (int i=0; i < num_decode_surfaces; i++) {
AVPacket *pkt = av_packet_alloc();
pkt->data = static_cast<u_int8_t *> (av_packet_data_[i].first);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you mean uint8_t? Not sure what u_int8_t is.


surface_stride_ = target_width_ * byte_per_pixel_;
chroma_height_ = static_cast<int>(std::ceil(target_height_ * GetChromaHeightFactor(video_surface_format_)));
chroma_width_ = (int)(ceil(target_width_ * GetChromaWidthFactor(video_surface_format_)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use static_cast here

AVPacket *pkt;
do {
pkt = PopPacket();
//std::cout << "pop packet" << std::endl;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove comment if unnecessary

while (status >= 0) {
status = avcodec_receive_frame(dec_context_, p_frame);
if (status == AVERROR(EAGAIN) || status == AVERROR_EOF) {
//av_frame_free(&p_frame);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove line if unnecessary

if (dec_context_->frame_number == 1) {
InitOutputFrameInfo(p_frame);
}
//std::cout<<"Decoding frame: " << dec_context_->frame_number << "<pframe, index>: " << p_frame << " " << av_frame_cnt_ << std::endl;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove line if unnecessary


/**
* @brief This function is used to get the current frame size based on pitch
*/
int GetFrameSizePitched() { assert(surface_stride_); return surface_stride_ * (disp_height_ + (chroma_height_ * num_chroma_planes_)); }
//int GetFrameSizePitched() { assert(surface_stride_); return surface_stride_ * (disp_height_ + (chroma_height_ * num_chroma_planes_)); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this commented out?

parser_params.clock_rate = clk_rate;
parser_params.max_display_delay = disp_delay_;
parser_params.user_data = this;
parser_params.pfn_sequence_callback = FFMpegVideoDecoder::FFMpegHandleVideoSequenceProc;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we call the functions directly since it's member functions of the same class. Do we need the scope?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:precheckin run mainline precheckin CI job enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants