Update patch set 2

Patch Set 2:

(4 comments)

Patch-set: 2
Attention: {"person_ident":"Gerrit User 1001562 \u003c1001562@3ce6091f-6c88-37e8-8c75-72f92ae8dfba\u003e","operation":"ADD","reason":"\u003cGERRIT_ACCOUNT_1115898\u003e replied on the change"}
Attention: {"person_ident":"Gerrit User 1115898 \u003c1115898@3ce6091f-6c88-37e8-8c75-72f92ae8dfba\u003e","operation":"REMOVE","reason":"\u003cGERRIT_ACCOUNT_1115898\u003e replied on the change"}
This commit is contained in:
Gerrit User 1115898 2026-07-27 13:40:04 -07:00 committed by Gerrit Code Review
parent 4fee320106
commit 081ab3488e

View File

@ -17,6 +17,23 @@
"revId": "407f21fe2985bc4a7a0c746e4e411bfe3a16af46",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": true,
"key": {
"uuid": "67be1114_3bb92ffb",
"filename": "include/libyuv/scale_row.h",
"patchSetId": 2
},
"lineNbr": 200,
"author": {
"id": 1115898
},
"writtenOn": "2026-07-27T20:40:04Z",
"side": 1,
"message": "internal functions should generally be static void. They do not need malloc or parameter checks because the calling function has already done this.",
"revId": "407f21fe2985bc4a7a0c746e4e411bfe3a16af46",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": true,
"key": {
@ -33,6 +50,58 @@
"message": "Frank, Tim: Can this `FixedDiv(height, uv_height)` call still overflow a signed int?\n\nI believe `FixDiv_C(height, uv_height)` is safe. What about `FixDiv_X86(height, uv_height)`?\n\n```\n// Divide num by div and return as 16.16 fixed point result.\nint FixedDiv_X86(int num, int div) {\n asm volatile(\n \"cdq \\n\"\n \"shld $0x10,%%eax,%%edx \\n\"\n \"shl $0x10,%%eax \\n\"\n \"idiv %1 \\n\"\n \"mov %0, %%eax \\n\"\n : \"+a\"(num) // %0\n : \"c\"(div) // %1\n : \"memory\", \"cc\", \"edx\");\n return num;\n}\n```",
"revId": "407f21fe2985bc4a7a0c746e4e411bfe3a16af46",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": true,
"key": {
"uuid": "493e92b1_e844beab",
"filename": "source/convert.cc",
"patchSetId": 2
},
"lineNbr": 297,
"author": {
"id": 1115898
},
"writtenOn": "2026-07-27T20:40:04Z",
"side": 1,
"message": "cdq sign extends to 64 bit\nshld+shl by 16 is 64 bit dividend being shifted left by 16 making it fixed point.\nidiv divides 64 bit dividend by 32 bit divisor.\nmov+return returns 32 bit result which is fixed 16.16\nthis function is equivalent to \n// Divide num by div and return as 16.16 fixed point result.\nint FixedDiv_C(int num, int div) {\n return (int)(((int64_t)(num) \u003c\u003c 16) / div);\n}\nIt will overflow if div\u003d1 and num \u003e\u003d 32768 which represents scaling an image down by a factor of 32768x",
"parentUuid": "a2cd09a2_97a36e30",
"revId": "407f21fe2985bc4a7a0c746e4e411bfe3a16af46",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": true,
"key": {
"uuid": "b87b7250_b3590de7",
"filename": "source/convert.cc",
"patchSetId": 2
},
"lineNbr": 302,
"author": {
"id": 1115898
},
"writtenOn": "2026-07-27T20:40:04Z",
"side": 1,
"message": "this isnt ideal. best have this function, I21xToI420 or its caller, check the params before this. ScalePlaneVertical_16To8 should never fail because the parameters have been checked.\n\npreferrably make a row function/planar function that does not use scaling.\niirc the intent of this function is blend 2 rows and convert 10 bits to 8 bits.\nThe x in the name I21xToI420 means it support 10, 12 or 16 bit.",
"revId": "407f21fe2985bc4a7a0c746e4e411bfe3a16af46",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
},
{
"unresolved": false,
"key": {
"uuid": "77884247_df53e5e0",
"filename": "source/scale.cc",
"patchSetId": 2
},
"lineNbr": 2101,
"author": {
"id": 1115898
},
"writtenOn": "2026-07-27T20:40:04Z",
"side": 1,
"message": "normally internal functions are \u0027void\u0027 but i guess we changed this one to have a return.",
"revId": "407f21fe2985bc4a7a0c746e4e411bfe3a16af46",
"serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba"
}
]
}