Derek Buitenhuis: > Signed-off-by: Derek Buitenhuis <derek.buitenh...@gmail.com> > --- > libavcodec/h2645_parse.c | 28 ++++++++++++++++++++++++++++ > libavcodec/hevcdec.c | 29 +++++++++++++++++++++++++++++ > libavcodec/hevcdec.h | 2 ++ > libavcodec/version.h | 2 +- > 4 files changed, 60 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c > index 6fbe97ad4a..7adf5b7ef1 100644 > --- a/libavcodec/h2645_parse.c > +++ b/libavcodec/h2645_parse.c > @@ -517,6 +517,34 @@ int ff_h2645_packet_split(H2645Packet *pkt, const > uint8_t *buf, int length, > pkt->nb_nals++; > } > > + /* > + * Due to limitions in avcodec's current frame threading code, it cannot > + * handle adding frame side data in any place except before the slice has > + * started decoding. Since Dolby Vision RPUs (which appear as NAL type > 62) > + * are appended to the AU, this is a poblematic. As a hack around this, > we > + * move any RPUs to before the first slice NAL. > + */ > + if (codec_id == AV_CODEC_ID_HEVC && pkt->nb_nals > 1 && > pkt->nals[pkt->nb_nals - 1].type == HEVC_NAL_UNSPEC62 &&
1. This code is also used CBS, not only the HEVC decoder. So unconditionally moving the NAL is unacceptable. (Does this type of NAL actually abide by the typical 0x03 escaping scheme? If not, CBS has a problem.) > + !pkt->nals[pkt->nb_nals - 1].nuh_layer_id && !pkt->nals[pkt->nb_nals > - 1].temporal_id) { > + > + int first_non_slice = 0; > + H2645NAL *tmp = av_malloc(pkt->nb_nals * sizeof(H2645NAL)); > + if (!tmp) > + return AVERROR(ENOMEM); > + > + for (int i = pkt->nb_nals - 1; i >= 0; i--) { > + if (pkt->nals[i].type < HEVC_NAL_VPS) > + first_non_slice = i; > + tmp[i] = pkt->nals[i]; > + } > + > + pkt->nals[first_non_slice] = pkt->nals[pkt->nb_nals - 1]; > + for (int i = first_non_slice + 1; i < pkt->nb_nals; i++) > + pkt->nals[i] = tmp[i - 1]; > + > + av_free(tmp); > + } 2. This is unnecessarily complicated: H2645NAL tmp = pkt->nals[pkt->nb_nals - 1]; memmove(&pkt->nals[first_non_slice + 1], &pkt->nals[first_non_slice], (pkt->nb_nals - 1 - first_non_slice) * sizeof(*pkt->nals)); pkt->nals[first_non_slice] = tmp; (The naming of first_non_slice is actually misleading: If there are slices, then it is the index of the first slice before moving. It would be easier to use an approach as follows: int dst_index; for (dst_index = 0; dst_index < pkt->nb_nals - 1; dst_index) if (pkt->nals[dst_index] < HEVC_NAL_VPS) break; ) > + > return 0; > } > > diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c > index 246ffd7d80..dd286deaa7 100644 > --- a/libavcodec/hevcdec.c > +++ b/libavcodec/hevcdec.c > @@ -2950,6 +2950,14 @@ static int set_side_data(HEVCContext *s) > } > } > > + if (s->rpu_buf) { > + AVFrameSideData *rpu = av_frame_new_side_data_from_buf(out, > AV_FRAME_DATA_DOVI_RPU_BUFFER, s->rpu_buf); > + if (!rpu) > + return AVERROR(ENOMEM); > + > + s->rpu_buf = NULL; > + } > + > return 0; > } > > @@ -3224,6 +3232,20 @@ static int decode_nal_unit(HEVCContext *s, const > H2645NAL *nal) > case HEVC_NAL_AUD: > case HEVC_NAL_FD_NUT: > break; > + case HEVC_NAL_UNSPEC62: > + /* > + * Check for RPU delimiter. > + * > + * Dolby Vision RPUs masquerade as unregistered NALs of type 62. > + */ > + if (nal->size > 2 && !nal->nuh_layer_id && !nal->temporal_id) { > + s->rpu_buf = av_buffer_alloc(nal->raw_size - 2); 3. This may leak in case there are multiple HEVC_NAL_UNSPEC62 in an H2645Packet. (Don't know whether this is legal, but it is unchecked.) 4. My preferred solution for this is adding the following after the call to ff_h2645_packet_split(): if (pkt->nb_nals > 1 && pkt->nals[pkt->nb_nals - 1].type == HEVC_NAL_UNSPEC62 && nal->size > 2 && !nal->nuh_layer_id && !nal->temporal_id) { // create rpu_buf here This is less of a hack than reordering the nalus. (This of course also needs a check to rule out leaks: After all, the preceding packet might have given us an rpu, but no first slice. Btw: That's the reason why this buffer is synced between threads, isn't it?) > + if (!s->rpu_buf) > + return AVERROR(ENOMEM); > + > + memcpy(s->rpu_buf->data, nal->raw_data + 2, nal->raw_size - 2); > + } > + break; > default: > av_log(s->avctx, AV_LOG_INFO, > "Skipping NAL unit %d\n", s->nal_unit_type); _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".