James Almer: > On 2/5/2024 1:18 PM, Andreas Rheinhardt wrote: >> James Almer: >>> On 2/5/2024 12:28 PM, Andreas Rheinhardt wrote: >>>> James Almer: >>>>> On 2/5/2024 12:12 PM, Andreas Rheinhardt wrote: >>>>>> James Almer: >>>>>>> On 2/3/2024 11:50 AM, Andreas Rheinhardt wrote: >>>>>>>>> diff --git a/libavformat/movenc.h b/libavformat/movenc.h >>>>>>>>> index 60363198c9..fee3e759e0 100644 >>>>>>>>> --- a/libavformat/movenc.h >>>>>>>>> +++ b/libavformat/movenc.h >>>>>>>>> @@ -25,7 +25,9 @@ >>>>>>>>> #define AVFORMAT_MOVENC_H >>>>>>>>> #include "avformat.h" >>>>>>>>> +#include "iamf.h" >>>>>>>>> #include "movenccenc.h" >>>>>>>>> +#include "libavcodec/bsf.h" >>>>>>>> >>>>>>>> There is no need to include these here, as you don't need complete >>>>>>>> types. This has the added benefit of forcing you to actually >>>>>>>> include >>>>>>>> the >>>>>>>> files where you are using them (namely in movenc.c, where you >>>>>>>> forgot to >>>>>>>> include bsf.h). >>>>>>> >>>>>>> Ok, fixed locally. >>>>>>> >>>>>>> Will push the set soon. >>>>>> >>>>>> It seems you have not noticed my objection to the first version of >>>>>> your set. >>>>>> >>>>>> - Andreas >>>>> >>>>> Can you link to it? >>>> >>>> Sorry, it was v2: >>>> https://ffmpeg.org/pipermail/ffmpeg-devel/2024-February/320722.html >>>> >>>> - Andreas >>> >>> I removed the codec list from the split bsf like you asked, and >>> explained what the bsfs do in the documentation. >> >> For those few codecs where different framings are common and supported >> by us, the muxers convert the given framing to the needs of the output >> format; decoders also support the various framings. This of course only >> works if they can decide which packetization the input uses; it is >> possible for the cases we support. >> >> If you allow that packets can contain OBU encapsulated data for >> arbitrary codec ids (even if only intended for a few of them), then this >> packetization would become officially allowed and we would have to adapt >> our muxers and decoders accordingly. Which is just not possible, because >> there is just no information available to base this decision on. > > So you want me to state that these bsfs should not be used at all by > library users, and that they are meant exclusively to be inserted by the > mov muxer and demuxer? >
Actually, I don't think that this should be done in a BSF at all. For the reasons outlined above. >> >> There is a second complication with iamf_frame_split_bsf: Up until now, >> BSFs only passed the stream index value through. But with this BSF the >> output may have multiple ids even when the input only had one. I am >> pretty sure that this will surprise and break many users. I don't know >> whether ffmpeg is among them (if a user inserts the BSF manually). > > This would be addressed by forbidding (or declaring unsupported) the > usage of the filters by external callers. > So a BSF for only one caller (lavf)? >> >> In fact, for this BSF the stream_index that the output packet gets is >> determined by the offset as well as the packet data alone. The only way >> for the demuxer to know these numbers is if it has already parsed the >> packet data before and added streams according to this. Looking at 3/6 > > I think you mean 4/6. > >> it seems that this is indeed what's happening (which is wasteful). But > > Packets are not being parsed, only the descriptors in the relevant mp4 > sample entry. > And it happens twice, which is wasteful. >> only partially: The iamf_descriptors data is checked and streams are >> created according to it, but the data read via av_append_packet() is not >> checked at all. What guarantees that it can't contain >> IAMF_OBU_IA_AUDIO_ELEMENT elements (which trigger adding new ids and >> therefore lead to an increment in the potential output stream_index)? >> Also notice that your 3/6 uses the pkt's stream_index coming out of the >> BSF without any check. > > Again 4/6. And i can add a check for stream_index. > I think we should never come in a scenario where this can happen. > I could make the split filter only parse descriptors once, and passing > them to it immediately after av_bfs_init(), so if packets have > descriptors, an error will be returned (The spec disallows descriptors > in samples). > There's also the bsf's input codecpar extradata. I could use that instead. If one were to use a BSF for this, then sending the descriptor via extradata would be the way to go. - Andreas _______________________________________________ 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".
