From 45bd581b7068062b0d1f9f5e609205e7380dd824 Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Thu, 14 Mar 2024 16:09:18 -0400 Subject: [PATCH] media/ffmpeg: Pass most packet metadata to decoder. While AVPacket itself contains other fields, the important ones are all static, and fit in 64 bytes. So we can put them into the chunk media header directly and then grab them again in AVCodecDecoder. With that passed through, we can then use the best_effort_timestamp, as it's more likely to be correct. Change-Id: Ied82137694f1307d4dcb177f3f9a7cb6b798461a Reviewed-on: https://review.haiku-os.org/c/haiku/+/7525 Reviewed-by: waddlesplash --- .../media/plugins/ffmpeg/AVCodecDecoder.cpp | 29 ++++++++----------- .../media/plugins/ffmpeg/AVFormatReader.cpp | 13 +++++++++ src/add-ons/media/plugins/ffmpeg/Utilities.h | 17 +++++++++-- 3 files changed, 40 insertions(+), 19 deletions(-) diff --git a/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.cpp b/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.cpp index d4dae159b8..cb79593759 100644 --- a/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.cpp +++ b/src/add-ons/media/plugins/ffmpeg/AVCodecDecoder.cpp @@ -1402,26 +1402,23 @@ AVCodecDecoder::_LoadNextChunkIfNeededAndAssignStartTime() fTempPacket->data = fChunkBuffer; fTempPacket->size = fChunkBufferSize; + fTempPacket->dts = chunkMediaHeader.start_time; // Let FFMPEG handle the correct relationship between start_time and // decoded a/v frame. By doing so we are simply copying the way how it // is implemented in ffplay.c for video frames (for audio frames it // works, too, but isn't used by ffplay.c). // \see http://git.videolan.org/?p=ffmpeg.git;a=blob;f=ffplay.c;h=09623db374e5289ed20b7cc28c262c4375a8b2e4;hb=9153b33a742c4e2a85ff6230aea0e75f5a8b26c2#l1502 - // - // FIXME: Research how to establish a meaningful relationship between - // start_time and decoded a/v frame when the received chunk buffer - // contains partial a/v frames. Maybe some data formats do contain time - // stamps (ake pts / dts fields) that can be evaluated by FFMPEG. But - // as long as I don't have such video data to test it, it makes no - // sense trying to implement it. - // - // FIXME: Implement tracking start_time of video frames originating in - // data chunks that encode more than one video frame at a time. In that - // case on would increment the start_time for each consecutive frame of - // such a data chunk (like it is done for audio frame decoding). But as - // long as I don't have such video data to test it, it makes no sense - // to implement it. + + if (chunkMediaHeader.user_data_type == AVPACKET_USER_DATA_TYPE) { + avpacket_user_data* data = (avpacket_user_data*)&chunkMediaHeader.user_data; + fTempPacket->pts = data->pts; + fTempPacket->dts = data->dts; + fTempPacket->stream_index = data->stream_index; + fTempPacket->flags = data->flags; + fTempPacket->duration = data->duration; + fTempPacket->pos = data->pos; + } #ifdef LOG_STREAM_TO_FILE BFile* logFile = fIsAudio ? &sAudioStreamLogFile : &sVideoStreamLogFile; @@ -1576,9 +1573,7 @@ AVCodecDecoder::_UpdateMediaHeaderForVideoFrame() fHeader.type = B_MEDIA_RAW_VIDEO; fHeader.file_pos = 0; fHeader.orig_size = 0; - fHeader.start_time = fRawDecodedPicture->pkt_dts; - // The pkt_dts is already in microseconds, even if ffmpeg docs says - // 'in codec time_base units' + fHeader.start_time = fRawDecodedPicture->best_effort_timestamp; fHeader.size_used = av_image_get_buffer_size( colorspace_to_pixfmt(fOutputColorSpace), fRawDecodedPicture->width, fRawDecodedPicture->height, 1); diff --git a/src/add-ons/media/plugins/ffmpeg/AVFormatReader.cpp b/src/add-ons/media/plugins/ffmpeg/AVFormatReader.cpp index c536c2649a..5bfeaecd90 100644 --- a/src/add-ons/media/plugins/ffmpeg/AVFormatReader.cpp +++ b/src/add-ons/media/plugins/ffmpeg/AVFormatReader.cpp @@ -693,6 +693,19 @@ StreamBase::GetNextChunk(const void** chunkBuffer, *chunkSize = fPacket.size; if (mediaHeader != NULL) { +#if __GNUC__ != 2 + static_assert(sizeof(avpacket_user_data) <= sizeof(mediaHeader->user_data), + "avpacket user data too large"); +#endif + mediaHeader->user_data_type = AVPACKET_USER_DATA_TYPE; + avpacket_user_data* data = (avpacket_user_data*)mediaHeader->user_data; + data->pts = fPacket.pts; + data->dts = fPacket.dts; + data->stream_index = fPacket.stream_index; + data->flags = fPacket.flags; + data->duration = fPacket.duration; + data->pos = fPacket.pos; + mediaHeader->type = fFormat.type; mediaHeader->buffer = 0; mediaHeader->destination = -1; diff --git a/src/add-ons/media/plugins/ffmpeg/Utilities.h b/src/add-ons/media/plugins/ffmpeg/Utilities.h index 0665a4c95d..790f95ead3 100644 --- a/src/add-ons/media/plugins/ffmpeg/Utilities.h +++ b/src/add-ons/media/plugins/ffmpeg/Utilities.h @@ -23,6 +23,19 @@ extern "C" { } +/*! \brief Structure used for passing AVPacket metadata through media_header::user_data. */ +struct avpacket_user_data { + int64_t pts; + int64_t dts; + int stream_index; + int flags; + int64_t duration; + int64_t pos; +}; + +#define AVPACKET_USER_DATA_TYPE 'ffav' + + /*! \brief Converts FFmpeg notation of video aspect ratio into the Media Kits notation. @@ -62,7 +75,7 @@ ConvertAVCodecContextToVideoAspectWidthAndHeight(AVCodecContext& contextIn, // ourselve based solely on the video dimensions av_reduce(&pixelAspectRatio.num, &pixelAspectRatio.den, contextIn.width, contextIn.height, 1024 * 1024); - + pixelWidthAspectOut = static_cast(pixelAspectRatio.num); pixelHeightAspectOut = static_cast(pixelAspectRatio.den); return; @@ -210,7 +223,7 @@ CalculateBytesPerRowWithColorSpaceAndVideoWidth(color_space colorSpace, int vide /*! \brief Converts the Media Kits notation of video frame rate to FFmpegs notation. - + \see ConvertAVCodecContextToVideoFrameRate() for converting in the other direction.