[jantonguirao]

Category:

Bug fix (non-breaking change which fixes an issue)

Description:

Two FFmpeg calls of the video decoders (FramesDecoderBase and its CPU and GPU implementations) were not safe from several threads at once:

• av_error_string formatted into a function-static buffer. It turns an FFmpeg error code into the text of DALI's error messages (opening a file, setting up the codec, sending packets to the decoder, converting frames, ...). When two threads fail at the same time, one thread can overwrite or clear the buffer while the other is still copying it into its std::string. The error message then shows the other thread's error, or a truncated/empty text. This is a data race on the error path, exactly when the message matters most.
• Every decoder constructor called av_log_set_level(AV_LOG_ERROR). FFmpeg's log level is a plain global (av_log_set_level only stores it), so decoders created on several threads wrote it concurrently, while FFmpeg's threads read it. The value is always the same, so the effect is benign, but it is still a data race, and it reset any level set in the meantime for every new decoder.

This already matters on main: fn.experimental.decoders.video creates its decoders and decodes its samples in parallel on a thread pool, one task per sample, so both are reached from several threads at once. Several video readers or decoders in one process (e.g. several pipelines) share them as well. It becomes more relevant with the parallel decoding in fn.experimental.readers.video planned in #6563 and #6564, where several decode lanes each create and use their own decoder in parallel.

The fix:

• av_error_string formats into a local, zero-initialized buffer of AV_ERROR_MAX_STRING_SIZE instead of a static one, so each call has its own storage.
• FFmpeg's log level is set once, by a static initializer in frames_decoder_base.cc that runs while the library is loaded, before any decoder exists, instead of by every decoder.

This PR is split out of #6545, which was too large to review as one change. It is based directly on main and does not depend on the other PRs of the split.

Additional information:

Affected modules and functionalities:

• dali/operators/video/video_utils.cc: av_error_string uses a local buffer instead of a function-static one.
• dali/operators/video/frames_decoder_base.cc: the log level is set by a static initializer (a lambda called once), no longer in the two FramesDecoderBase constructors.

Affects every operator built on these decoders: fn.experimental.readers.video, fn.experimental.decoders.video and fn.experimental.inputs.video, CPU and GPU.

Key points relevant for the review:

• Error message texts and the log level DALI sets are the same.
• FFmpeg's log level becomes AV_LOG_ERROR when DALI's operators library is loaded (e.g. import nvidia.dali), not when the first video decoder is created. This also applies to other FFmpeg users in the process (PyAV, torchvision, ...) from that point. A level set later is kept: DALI no longer resets it for every new decoder.
• The legacy video reader's loader still sets the same level from its own initialization; it is not changed here.

Tests:

• Existing tests apply
  • dali/test/python/decoder/test_video.py (reader and decoder tests, CPU and GPU, including tests that expect decoding errors)
  • dali/operators/video/frames_decoder_test.cc
• New tests added
  • Python tests
  • GTests
  • Benchmark
  • Other
• N/A

Checklist

Documentation

• Existing documentation applies
• Documentation updated
  • Docstring
  • Doxygen
  • RST
  • Jupyter
  • Other
• N/A

DALI team only

Requirements

• Implements new requirements
• Affects existing requirements
• N/A

REQ IDs: N/A

JIRA TASK: DALI-4935

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[Copilot]

🟢 Approval recommended

The localized changes use appropriate storage and synchronization, with no identified blocking issues; tests were not run.

No open issues 🎉

What changed in this PR

This PR addresses two concurrency issues in the FFmpeg helpers shared by DALI’s CPU and GPU video decoders, supporting existing parallel decoding and planned video-reader improvements.

Changes:

• Use a local buffer for FFmpeg error strings to prevent concurrent calls from overwriting each other.
• Initialize FFmpeg’s log level through std::call_once, shared by both decoder constructors.

File	Description
dali/​operators/​video/​video_utils.cc	Replaces the shared error-message buffer with per-call storage.
dali/​operators/​video/​frames_decoder_base.cc	Adds synchronized, one-time log-level initialization for both constructors.

_{Review effort: Balanced}

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable new issues were found.

Summary

Makes video decoder calls safer when several threads run at once.

• Video decoder errors keep their own text during parallel failures.
• DALI sets FFmpeg’s error log level once when its library loads.

_{Reviews (2) · Last reviewed commit: "Set FFmpeg's log level once, when the li..." · Reviewed by Greptile}

[mzient]

This likely costs more than just calling av_log_set_level every time. The function is defined as

void av_log_set_level(int level)
{
    av_log_level = level;
}

and it prevents the value from changing, should we ever want to alter the log level while already running.

[jantonguirao]

Agreed, call_once costs more than the store it protects, and it would keep the level from being changed later. I removed it, and the level is now set once by a static initializer in this file, while the library is loaded and before any decoder exists (5b4cb1f). That also removes the concurrent writes from decoders created on several threads, and a level set later is no longer reset by every new decoder.

[jantonguirao]

!build

[dali-automaton]

CI MESSAGE: [77048268]: BUILD STARTED

[dali-automaton]

CI MESSAGE: [77048268]: BUILD PASSED