| Message ID | 20231218145859.1162669-1-petr.matousek@comprimato.com |
|---|---|
| Headers | show |
| Series | fftools/ffprobe: dump contents of the AV_FRAME_DATA_SEI_UNREGISTERED | expand |
On Mon, 18 Dec 2023 at 14:59, Petr Matousek <petr.matousek@comprimato.com> wrote: > Before this patch being applied the ffprobe just tells the user whether > the H.26[45] User Data Unregistered SEI message is present in the frame > side data > or not. After the patch being appliend it also dumps the contents of the > data > similar way as for the other already supported frame side data types. > > Petr Matousek (1): > fftools/ffprobe: dump contents of the AV_FRAME_DATA_SEI_UNREGISTERED > I don't think ffprobe should be dumping potentially large amounts of random SEI data to the terminal. Some encoders use this space for their own data and it's annoying to see it spam the terminal. Kieran
On date Monday 2023-12-18 14:58:58 +0000, Petr Matousek wrote: > Before this patch being applied the ffprobe just tells the user whether > the H.26[45] User Data Unregistered SEI message is present in the frame side data > or not. After the patch being appliend it also dumps the contents of the data appliend typo > similar way as for the other already supported frame side data types. What's the use case for this? I wonder if we should employ some more generic mechanism, e.g. a filter moving side data to metadata for display.
Dne 19/12/2023 v 00:01 Stefano Sabatini napsal(a): > On date Monday 2023-12-18 14:58:58 +0000, Petr Matousek wrote: >> Before this patch being applied the ffprobe just tells the user whether >> the H.26[45] User Data Unregistered SEI message is present in the frame side data > >> or not. After the patch being appliend it also dumps the contents of the data > > appliend typo > >> similar way as for the other already supported frame side data types. > > What's the use case for this? > > I wonder if we should employ some more generic mechanism, e.g. a > filter moving side data to metadata for display. We needed to verify that our implementation of the MISB ST 0604 produces correct contents of the SEI data. Using ffprobe for that was very useful as we were able to pair the SEI data with other frame data like the PTS etc.
On date Monday 2023-12-18 19:06:10 +0000, Kieran Kunhya wrote: > On Mon, 18 Dec 2023 at 14:59, Petr Matousek <petr.matousek@comprimato.com> > wrote: > > > Before this patch being applied the ffprobe just tells the user whether > > the H.26[45] User Data Unregistered SEI message is present in the frame > > side data > > or not. After the patch being appliend it also dumps the contents of the > > data > > similar way as for the other already supported frame side data types. > > > > Petr Matousek (1): > > fftools/ffprobe: dump contents of the AV_FRAME_DATA_SEI_UNREGISTERED > > > > I don't think ffprobe should be dumping potentially large amounts of random > SEI data to the terminal. Some encoders use this space for their own data > and it's annoying to see it spam the terminal. I'm not super concerned by this, since you can filter out the required information. Also I don't see other viable alternatives to show this information (moving this to metadata would require decoding).
On Tue, 2 Jan 2024 at 11:27, Stefano Sabatini <stefasab@gmail.com> wrote: > On date Monday 2023-12-18 19:06:10 +0000, Kieran Kunhya wrote: > > On Mon, 18 Dec 2023 at 14:59, Petr Matousek < > petr.matousek@comprimato.com> > > wrote: > > > > > Before this patch being applied the ffprobe just tells the user whether > > > the H.26[45] User Data Unregistered SEI message is present in the frame > > > side data > > > or not. After the patch being appliend it also dumps the contents of > the > > > data > > > similar way as for the other already supported frame side data types. > > > > > > Petr Matousek (1): > > > fftools/ffprobe: dump contents of the AV_FRAME_DATA_SEI_UNREGISTERED > > > > > > > > I don't think ffprobe should be dumping potentially large amounts of > random > > SEI data to the terminal. Some encoders use this space for their own data > > and it's annoying to see it spam the terminal. > > I'm not super concerned by this, since you can filter out the required > information. Also I don't see other viable alternatives to show this > information (moving this to metadata would require decoding). > How are you able to filter between different types of unregistered SEI? This is yet another thing that needs to be done with the API and not the CLI (something this project is unable to understand). Kieran
On date Tuesday 2024-01-02 11:34:40 +0000, Kieran Kunhya wrote: > On Tue, 2 Jan 2024 at 11:27, Stefano Sabatini <stefasab@gmail.com> wrote: > > > On date Monday 2023-12-18 19:06:10 +0000, Kieran Kunhya wrote: > > > On Mon, 18 Dec 2023 at 14:59, Petr Matousek < > > petr.matousek@comprimato.com> > > > wrote: > > > > > > > Before this patch being applied the ffprobe just tells the user whether > > > > the H.26[45] User Data Unregistered SEI message is present in the frame > > > > side data > > > > or not. After the patch being appliend it also dumps the contents of > > the > > > > data > > > > similar way as for the other already supported frame side data types. > > > > > > > > Petr Matousek (1): > > > > fftools/ffprobe: dump contents of the AV_FRAME_DATA_SEI_UNREGISTERED > > > > > > > > > > > > I don't think ffprobe should be dumping potentially large amounts of > > random > > > SEI data to the terminal. Some encoders use this space for their own data > > > and it's annoying to see it spam the terminal. > > > > I'm not super concerned by this, since you can filter out the required > > information. Also I don't see other viable alternatives to show this > > information (moving this to metadata would require decoding). > > > > How are you able to filter between different types of unregistered SEI? At the moment you can opt-in what must be shown (opt-out is not an option). But from the point of view of the side data, probably there is no way to filter that out depending on the type. But this can be done later by processing the data (which is mostly the point of the output generated by ffprobe). This is the OP use case after all: |We needed to verify that our implementation of the MISB ST 0604 produces |correct contents of the SEI data. Using ffprobe for that was very useful as we |were able to pair the SEI data with other frame data like the PTS etc. > This is yet another thing that needs to be done with the API I don't understand what you mean by "the API". Provided that the API exposes the information about the side data type, but not much else at this level, you still would probably need to postprocess the data. > and not the CLI (something this project is unable to understand). What is exposed by the API is exercised by the CLI (which is a good thing for scripting and for testing).
> > > and not the CLI (something this project is unable to understand). > > What is exposed by the API is exercised by the CLI (which is a good > thing for scripting and for testing). > The CLI can't just write random binary data to the terminal. What next, raw frames available via JSON? I'm sure there are users who want that too. Again, this is something that should either be parsed by FFmpeg or done via the API. Kieran
On Tue, Jan 2, 2024 at 4:06 PM Kieran Kunhya <kierank@obe.tv> wrote: > > > > > and not the CLI (something this project is unable to understand). > > > > What is exposed by the API is exercised by the CLI (which is a good > > thing for scripting and for testing). > > > > The CLI can't just write random binary data to the terminal. What next, raw > frames available via JSON? I'm sure there are users who want that too. > Again, this is something that should either be parsed by FFmpeg or done via > the API. > +1
On date Tuesday 2024-01-02 21:05:47 +0000, Kieran Kunhya wrote: > > > > > and not the CLI (something this project is unable to understand). > > > > What is exposed by the API is exercised by the CLI (which is a good > > thing for scripting and for testing). > > > > The CLI can't just write random binary data to the terminal. > What next, raw frames available via JSON? I'm sure there are users > who want that too. This is already available through the -show_data option since years (but note that the binary data is rendered as a data dump). My point is that we might opt-out the verbose dump if we have a clear criterion for opting-in/out. For example -show_data is off by default, and we could mark special side-data fields with some flags to opt them out by default. > Again, this is something that should either be parsed by FFmpeg or done via > the API. I still don't understand. The CLI exposes what the API does, so if the API can't do it, the CLI cannot do either. "Parsed by FFmpeg" sounds like you are proposing to extend the FFmpeg API to parse some more information (about the SEI subtype?) and make it available through some more specific SEI side data?
> > My point is that we might opt-out the verbose dump if we have a clear > criterion for opting-in/out. For example -show_data is off by default, > and we could mark special side-data fields with some flags to opt them > out by default. Again, for unregistered SEI you don't have a choice. Some encoders put tons of private data in there. Some of the stuff is human readable, some of it isn't. > > Again, this is something that should either be parsed by FFmpeg or done > via > > the API. > > I still don't understand. The CLI exposes what the API does, so if the > API can't do it, the CLI cannot do either. The OP wants to test an implementation they have and they should do it using the API, not using ffprobe to test their third party implementation of something. "Parsed by FFmpeg" sounds like you are proposing to extend the FFmpeg > API to parse some more information (about the SEI subtype?) and make > it available through some more specific SEI side data? > As the OP says, this SEI data is from "MISB ST 0604" which is something that could be implemented in FFmpeg and parsed properly like all the other side data instead of just writing random binary data to the terminal. I don't understand why any of this is hard to comprehend. Kieran
On date Tuesday 2024-01-02 22:08:54 +0000, Kieran Kunhya wrote: [...] > The OP wants to test an implementation they have and they should do it > using the API, not using ffprobe to test their third party implementation > of something. > > > "Parsed by FFmpeg" sounds like you are proposing to extend the FFmpeg > > API to parse some more information (about the SEI subtype?) and make > > it available through some more specific SEI side data? > > > > As the OP says, this SEI data is from "MISB ST 0604" which is something > that could be implemented in FFmpeg and parsed properly like all the other > side data instead of just writing random binary data to the terminal. Fine, so you are proposing to extend the parser to put this information in the side data. This makes sense to me and sounds a better approach than the original proposal. @Petr can you update the patch to extend the parser to expose this information? Feel free to ask in case something is not clear (I'm not familiar myself with that part of the code, but I can try to help).
On Wed, 3 Jan 2024 at 15:44, Stefano Sabatini <stefasab@gmail.com> wrote: > On date Tuesday 2024-01-02 22:08:54 +0000, Kieran Kunhya wrote: > [...] > > The OP wants to test an implementation they have and they should do it > > using the API, not using ffprobe to test their third party implementation > > of something. > > > > > > "Parsed by FFmpeg" sounds like you are proposing to extend the FFmpeg > > > API to parse some more information (about the SEI subtype?) and make > > > it available through some more specific SEI side data? > > > > > > > As the OP says, this SEI data is from "MISB ST 0604" which is something > > that could be implemented in FFmpeg and parsed properly like all the > other > > side data instead of just writing random binary data to the terminal. > > Fine, so you are proposing to extend the parser to put this > information in the side data. This makes sense to me and sounds a > better approach than the original proposal. > SEIs are reordered so they need to go through the decoder to be frame accurate. I don't believe the parser is enough. Kieran