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.

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

Removed downscaling, which made the image quality worse in the video … #4452

Closed

Conversation

evekeen
Copy link
Contributor

@evekeen evekeen commented Mar 13, 2022

A naive attempt to fix image quality for imported tasks: #4377

@evekeen evekeen requested a review from nmanovic as a code owner March 13, 2022 21:00
@nmanovic nmanovic requested review from Marishka17 and azhavoro March 14, 2022 06:39
@nmanovic
Copy link
Contributor

@Marishka17 , @azhavoro , please look at comment.

@evekeen
Copy link
Contributor Author

evekeen commented Mar 14, 2022

That's actually a nasty bug, which made the whole dataset look incorrect, since the work was done with down-scaled images, while exported dataset is not downscaled. And objects can be in different locations because of the quality loss.
The issue appears if you create tasks through CLI - they're downscaled

@Marishka17
Copy link
Contributor

@evekeen, Hello, thanks for the contribution. I'm not sure this is the correct solution.
I was able to reproduce the error when creating a task with a *.mov video file using the use_zip_chunks=True flag.
In this case, the ZipCompressedChunkWriter class is used to create a compressed chunk on the server.
If we create a backup of this task and import it, the Mpeg4CompressedChunkWriter will be used on the server.
I guess the problem with different quality is in this.

@evekeen
Copy link
Contributor Author

evekeen commented Mar 15, 2022

@Marishka17 how do you suggest to fix that?

@Marishka17
Copy link
Contributor

As far as I remember the solution in PR was implemented without saving use_zip_chunks in task.json. When a task is creating this check defines a chunk type, but in buckup this option is missing.
So I guess we need to save this metadata when creating a task backup.

@Marishka17
Copy link
Contributor

@azhavoro, Do you have other suggestions?

@evekeen
Copy link
Contributor Author

evekeen commented Mar 19, 2022

@Marishka17 I've just tried settings that option in my task.json, and it was ignored on import:
WARNING cvat.server: the following keys are dropped {'use_zip_chunks'}
Moreover, I see in the code we actually set use_zip_chunks programmatically during the import

@evekeen
Copy link
Contributor Author

evekeen commented Mar 19, 2022

I've supported the use_zip_chunks option on import. However, if you export a task again, it will be exported as an imageset with use_zip_chunks=False. I hope you can take it from here.

Also, I suggest keeping the initial change of downscaling, since I don't see a reason why downscale

@evekeen evekeen force-pushed the feature/fixed-quality-of-imported-tasks branch from c941933 to 602cf04 Compare March 19, 2022 16:44
@Marishka17
Copy link
Contributor

Hi, @evekeen, thanks for the contribution! Are you still going to contribute to us? If so, could you please resolve conflicts in this PR?

@Marishka17
Copy link
Contributor

@azhavoro, Could you please take a look at the PR? I've resolved conflicts and updated it. PR should resolve issue #4377

@Marishka17 Marishka17 added the bug Something isn't working label Sep 8, 2022
@azhavoro azhavoro self-assigned this Sep 19, 2022
@nmanovic
Copy link
Contributor

nmanovic commented Sep 21, 2022

@evekeen , it looks like most parts of your PR has been merged. I see no changes in the PR. Thank you for the contribution.

@nmanovic nmanovic closed this Sep 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants