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

[x265] Lossless option #515

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[x265] Lossless option #515

wants to merge 3 commits into from

Conversation

szlldm
Copy link
Contributor

@szlldm szlldm commented Apr 14, 2024

No description provided.

@eumagga0x2a
Copy link
Collaborator

eumagga0x2a commented Apr 14, 2024

Did you try to close the video after encoding? I guess the ADM_paramList.cpp part is missing:

index dc2a65bc3..27caccc96 100644
--- a/avidemux_core/ADM_coreUtils/src/ADM_paramList.cpp
+++ b/avidemux_core/ADM_coreUtils/src/ADM_paramList.cpp
@@ -159,6 +159,11 @@ static bool compressReadFromString(COMPRES_PARAMS *params,const char *str)
         params->mode=COMPRESS_SAME;
         return true;
     }
+    if(!strcasecmp(str,"LOSSLESS"))
+    {
+        params->mode=COMPRESS_LOSSLESS;
+        return true;
+    }
     // all other are in the form a=b
     strcpy(tmp,str);
     char *s=tmp;
@@ -197,8 +202,9 @@ bool ADM_compressWriteToString(COMPRES_PARAMS *params,  char **str)
         case COMPRESS_SAME:  sprintf(tmp,"SAME");break;
         case COMPRESS_2PASS_BITRATE: sprintf(tmp,"2PASSBITRATE=%" PRIu32,params->avg_bitrate);break;
         case COMPRESS_AQ:    sprintf(tmp,"AQ=%" PRIu32,params->qz);break;
+        case COMPRESS_LOSSLESS: sprintf(tmp,"LOSSLESS");break;
         default:
-            ADM_error("Unknown compressin mode \n");
+            ADM_error("Unknown compression mode\n");
             return false;
     }
     *str=ADM_strdup(tmp);

@szlldm
Copy link
Contributor Author

szlldm commented Apr 14, 2024

Thanks. It just needed a minor tweak to work properly.

@@ -197,8 +198,9 @@ bool ADM_compressWriteToString(COMPRES_PARAMS *params, char **str)
case COMPRESS_SAME: sprintf(tmp,"SAME");break;
case COMPRESS_2PASS_BITRATE: sprintf(tmp,"2PASSBITRATE=%" PRIu32,params->avg_bitrate);break;
case COMPRESS_AQ: sprintf(tmp,"AQ=%" PRIu32,params->qz);break;
case COMPRESS_LOSSLESS: sprintf(tmp,"LOSSLESS=1");break;
Copy link
Collaborator

Choose a reason for hiding this comment

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

For me, it feels somewhat awkward to write "general.params=LOSSLESS=1" when "general.params=LOSSLESS=0" means exactly the same. Could you please explain the rationale?

With "general.params=LOSSLESS" we can exit compressReadFromString() early like in case of "general.params=SAME" (just not really used anywhere) and it is free from ambiguity, IMHO.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In my Q&D version to try out the lossless mode, I ended up with the same changes here, but, for a proper implementation, I'd love to see all options overridden by lossless mode systematically disabled. Maybe not trying to rush it in 2.8.2 but doing it thoroughly right thereafter?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants