Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Workaround for av context closing issue when using AUTO thread_type #8555

Merged
merged 2 commits into from
Oct 18, 2024
Merged

Workaround for av context closing issue when using AUTO thread_type #8555

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4ddf0ea
Workaround for av context closing issue when using AUTO thread_type
azhavoro Oct 17, 2024
9f2cacb
Add changelog note
azhavoro Oct 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelog.d/20241017_155815_andrey_fix_task_creating.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
### Fixed

- av context closing issue when using AUTO thread_type
(<https://github.com/cvat-ai/cvat/pull/8555>)
8 changes: 7 additions & 1 deletion cvat/apps/engine/media_extractors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,8 @@ def read_av_container(self, source: Union[str, io.BytesIO]) -> av.container.Inpu
for stream in container.streams:
context = stream.codec_context
if context and context.is_open:
# Currently, context closing may get stuck on some videos for an unknown reason,
# so the thread_type == 'AUTO' setting is disabled for future investigation
context.close()

if container.open_files:
Expand Down Expand Up @@ -583,7 +585,7 @@ def __init__(
stop: Optional[int] = None,
dimension: DimensionType = DimensionType.DIM_2D,
*,
allow_threading: bool = True,
allow_threading: bool = False,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

⚠️ Potential issue

Issue Found: VideoReader Instantiation Missing allow_threading Parameter

An instance of VideoReader is instantiated without explicitly setting allow_threading, which may lead to unintended side effects due to the default value change.

  • cvat/apps/engine/migrations/0024_auto_20191023_1025.py:68: 
    reader = VideoReader([original_video], get_frame_step(frame_filter), start_frame, _stop_frame)
🔗 Analysis chain

Changing default allow_threading to False may introduce unintended side effects

Changing the default value of allow_threading from True to False might impact existing functionality that relies on threading being enabled by default. Please verify that all usages of the VideoReader class are reviewed and updated if necessary to prevent any unintended behavior.

Run the following script to identify instances where VideoReader is instantiated without explicitly setting allow_threading:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find all instantiations of VideoReader without 'allow_threading' parameter specified.

# Expected Output: List of files and line numbers where 'VideoReader' is instantiated without 'allow_threading'.

ast-grep --lang python --pattern $'VideoReader($_, $_, $_, $_, $_)'

Length of output: 68

Script:

#!/bin/bash
# Description: Find all instantiations of VideoReader without 'allow_threading' parameter specified, regardless of the number of parameters.

# Expected Output: List of files and line numbers where 'VideoReader' is instantiated without 'allow_threading'.

ast-grep --lang python --pattern $'VideoReader($_, $$$)' | \
  grep -v 'allow_threading='

Length of output: 257

):
super().__init__(
source_path=source_path,
Expand Down Expand Up @@ -635,6 +637,8 @@ def iterate_frames(

if self.allow_threading:
video_stream.thread_type = 'AUTO'
else:
video_stream.thread_type = 'NONE'

frame_counter = itertools.count()
with closing(self._decode_stream(container, video_stream)) as stream_decoder:
Expand Down Expand Up @@ -795,6 +799,8 @@ def iterate_frames(self, *, frame_filter: Iterable[int]) -> Iterable[av.VideoFra
video_stream = container.streams.video[0]
if self.allow_threading:
video_stream.thread_type = 'AUTO'
else:
video_stream.thread_type = 'NONE'

container.seek(offset=start_decode_timestamp, stream=video_stream)

Expand Down
Loading