| Message ID | 20210802205501.1792236-1-derek.buitenhuis@gmail.com |
|---|---|
| State | Accepted |
| Commit | e6754d2ad20c918735d3bf5cc8122ac57f53d03c |
| Headers | show |
| Series | [FFmpeg-devel] ffprobe: Rename Audio Service Type 'type' field to 'service_type' | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
On 8/2/2021 9:55 PM, Derek Buitenhuis wrote: > --- > fftools/ffprobe.c | 2 +- > tests/ref/fate/hls-fmp4_ac3 | 2 +- Ping. - Derek
On 8/4/2021 4:25 PM, Derek Buitenhuis wrote: >> --- >> fftools/ffprobe.c | 2 +- >> tests/ref/fate/hls-fmp4_ac3 | 2 +- > > Ping. Will push tomorrow if nobody objects. - Derek
diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c index f411ba35b5..9165523b29 100644 --- a/fftools/ffprobe.c +++ b/fftools/ffprobe.c @@ -2032,7 +2032,7 @@ static void print_pkt_side_data(WriterContext *w, print_int("dv_bl_signal_compatibility_id", dovi->dv_bl_signal_compatibility_id); } else if (sd->type == AV_PKT_DATA_AUDIO_SERVICE_TYPE) { enum AVAudioServiceType *t = (enum AVAudioServiceType *)sd->data; - print_int("type", *t); + print_int("service_type", *t); } else if (sd->type == AV_PKT_DATA_MPEGTS_STREAM_ID) { print_int("id", *sd->data); } else if (sd->type == AV_PKT_DATA_CPB_PROPERTIES) { diff --git a/tests/ref/fate/hls-fmp4_ac3 b/tests/ref/fate/hls-fmp4_ac3 index ce7367de4d..cbe3ea1f1f 100644 --- a/tests/ref/fate/hls-fmp4_ac3 +++ b/tests/ref/fate/hls-fmp4_ac3 @@ -5,6 +5,6 @@ channels=6 channel_layout=5.1(side) [SIDE_DATA] side_data_type=Audio Service Type -type=0 +service_type=0 [/SIDE_DATA] [/STREAM]
677a030b26045acb50353d7954ae984ceefcd807 introduced more printable side data types in ffprobe, however the Audio Service Type side data 'type' field that was introduced aliases an existing field of the same name within the side data array, which can lead to JSON output like: "side_data_list": [ { "side_data_type": "Audio Service Type", "type": 0 }, { "side_data_type": "Stereo 3D", "type": "side by side", "inverted": 1 } ] This, while technically valid JSON, is considered bad practice, since it forces all downstream users to manually parse it and check all types; it makes simple deserialization impossible. Worse, in som loosely type languages, it can lead to silent bugs if exising code assumed it was a different type. As such, rename this second "type" field to "service_type". Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com> --- Not sure if this is considered a "break"? I am open to other ideas, but I do thinking leaving it as-is is not a very good idea - JSON should be simply deserializable. --- fftools/ffprobe.c | 2 +- tests/ref/fate/hls-fmp4_ac3 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)