5 ms·

FFmpeg is not happy with AI generated patches sent by AMD

40 points by itvision 8mo ago

estimator7292 8mo ago
The xitter post consists of only a screenshot, a link, and a drive-by comment. Here's the actual thing we're talking about https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21595#issuecomment-23753 https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21595#issuecomme...
guilhas 8mo ago
AI code, junior dev or distracted person, either way there is no defense for AMD. As a billion dollar company you would at least have a simple QA before pushing slop into open source projects FFmpeg maintainers have to keep pointing out incorrect parts. And conclude the entire is redundant The PR interaction are clearly AI filler, at least on the second one At this point open source projects should charge big companies to review their PRs
dryarzeg 8mo ago
const int EIGHT = 8 lol I really doubt any AI (even some small local models) would actually generate something like this :)
- hulitu 8mo ago
  > const int EIGHT = 8 lol > “The primary purpose of the DATA statement is to give names to constants; instead of referring to pi as 3.141592653589793 at every appearance, the variable PI can be given that value with a DATA statement and used instead of the longer form of the constant. This also simplifies modifying the program, should the value of pi change.” > — Early FORTRAN manual for Xerox Computers, attributed to David H. Owens.
- Alifatisk 8mo ago
  Yeah, I don’t think this is a case of ai slop. LLMs tend to be verbose with the comments but are fine with magic constants, at least from my experience.
- SomeUserName432 8mo ago
  I've run into something akin to `const int EIGHT = 7`. Courtesy of TCS.
- bobx11 8mo ago
  Agreed. This reads more like a very junior dev reads static analysis warnings and extracts it into a constant to satisfy the ide. An LLM would at least give the constant a slightly abstracted name.
Alifatisk 8mo ago
The dialog was something to read. Doesn’t sound like ai but rather something a developer has practiced for years and no one has challenged the dev on this, until now. Also, there is discussions on what a commit message should contain, apparently the patch had user guidance in the commit message. Literally on how to install dependencies such as gcc using pacman, in the commit message. https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21595 https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21595
- yesco 8mo ago
  I get maintainers have their own issues to deal with, and respect that they are trying to keep the project clean. At work I have had many times where I spent more of my day reviewing MRs than actually writing code, and sometimes my cold blunt replies can unintentionally rub people the wrong way. Still, I feel like they were pretty rude to this guy for no real reason. I don't think I'd want to work with them.
high_na_euv 8mo ago
I dont see problem with user manual in commit as long as reasoning for the commit is clearly written
- guilhas 8mo ago
  -Where are the build instructions? -Please go check that commit message from 2011
- emsign 7mo ago
  It's not about the commit itself but that due to it they discovered vibe coding by AMD. They object to the sending commits that have not been checked by a human dev.
- mort96 8mo ago
  Really? You think a git commit message in a C project is the right place to document how to install make and gcc on arch linux? That's not better suited as part of a readme file?
  - high_na_euv 8mo ago
    It should be in readme but I wouldn't complain if commit with new feature also had it
    - hulitu 8mo ago
      > if commit with new feature also had it Maybe reading his OS documentation will be more appropiate. What's next ? How to install Arch ? Or Mesa ?
      - emsign 7mo ago
        But but but it could be helpful for the next coding agent in case how to use a compiler has already fallen out of its context window! Stay positive, man./s
    - deleted 8mo ago
      [deleted]

40 points by itvision 8mo ago

The xitter post consists of only a screenshot, a link, and a drive-by comment. Here's the actual thing we're talking about https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21595#issuecomment-23753 https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21595#issuecomme...

guilhas 8mo ago

AI code, junior dev or distracted person, either way there is no defense for AMD. As a billion dollar company you would at least have a simple QA before pushing slop into open source projects FFmpeg maintainers have to keep pointing out incorrect parts. And conclude the entire is redundant The PR interaction are clearly AI filler, at least on the second one At this point open source projects should charge big companies to review their PRs

dryarzeg 8mo ago

const int EIGHT = 8 lol I really doubt any AI (even some small local models) would actually generate something like this :)

hulitu 8mo ago
> const int EIGHT = 8 lol > “The primary purpose of the DATA statement is to give names to constants; instead of referring to pi as 3.141592653589793 at every appearance, the variable PI can be given that value with a DATA statement and used instead of the longer form of the constant. This also simplifies modifying the program, should the value of pi change.” > — Early FORTRAN manual for Xerox Computers, attributed to David H. Owens.
Alifatisk 8mo ago
Yeah, I don’t think this is a case of ai slop. LLMs tend to be verbose with the comments but are fine with magic constants, at least from my experience.
SomeUserName432 8mo ago
I've run into something akin to `const int EIGHT = 7`. Courtesy of TCS.
bobx11 8mo ago
Agreed. This reads more like a very junior dev reads static analysis warnings and extracts it into a constant to satisfy the ide. An LLM would at least give the constant a slightly abstracted name.

Alifatisk 8mo ago

The dialog was something to read. Doesn’t sound like ai but rather something a developer has practiced for years and no one has challenged the dev on this, until now. Also, there is discussions on what a commit message should contain, apparently the patch had user guidance in the commit message. Literally on how to install dependencies such as gcc using pacman, in the commit message. https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21595 https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21595

yesco 8mo ago
I get maintainers have their own issues to deal with, and respect that they are trying to keep the project clean. At work I have had many times where I spent more of my day reviewing MRs than actually writing code, and sometimes my cold blunt replies can unintentionally rub people the wrong way. Still, I feel like they were pretty rude to this guy for no real reason. I don't think I'd want to work with them.

high_na_euv 8mo ago

I dont see problem with user manual in commit as long as reasoning for the commit is clearly written

guilhas 8mo ago
-Where are the build instructions? -Please go check that commit message from 2011
emsign 7mo ago
It's not about the commit itself but that due to it they discovered vibe coding by AMD. They object to the sending commits that have not been checked by a human dev.
mort96 8mo ago
Really? You think a git commit message in a C project is the right place to document how to install make and gcc on arch linux? That's not better suited as part of a readme file?
- high_na_euv 8mo ago
  It should be in readme but I wouldn't complain if commit with new feature also had it
  - hulitu 8mo ago
    > if commit with new feature also had it Maybe reading his OS documentation will be more appropiate. What's next ? How to install Arch ? Or Mesa ?
    - emsign 7mo ago
      But but but it could be helpful for the next coding agent in case how to use a compiler has already fallen out of its context window! Stay positive, man./s
  - deleted 8mo ago
    [deleted]